From 36902023d02d8bf3aec6564e43dedb8c0269ec25 Mon Sep 17 00:00:00 2001 From: Oussama Hassine Date: Tue, 24 Sep 2024 17:35:24 +0200 Subject: [PATCH] fix: crash on starting foreground service for calling --- .../com/wire/android/services/CallService.kt | 3 ++ .../wire/android/services/ServicesManager.kt | 4 +- .../android/services/ServicesManagerTest.kt | 43 +++++++++++++------ 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/app/src/main/kotlin/com/wire/android/services/CallService.kt b/app/src/main/kotlin/com/wire/android/services/CallService.kt index 5acf885cd6f..087a77ac960 100644 --- a/app/src/main/kotlin/com/wire/android/services/CallService.kt +++ b/app/src/main/kotlin/com/wire/android/services/CallService.kt @@ -45,6 +45,7 @@ import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flowOf @@ -121,6 +122,8 @@ class CallService : Service() { .distinctUntilChanged() .flatMapRight { callData -> callNotificationManager.reloadIfNeeded(callData) + }.debounce { + if (it is Either.Left) ServicesManager.DEBOUNCE_TIME else 0L } .collectLatest { it.fold( diff --git a/app/src/main/kotlin/com/wire/android/services/ServicesManager.kt b/app/src/main/kotlin/com/wire/android/services/ServicesManager.kt index 595b767ab5d..5a3aa8bdf13 100644 --- a/app/src/main/kotlin/com/wire/android/services/ServicesManager.kt +++ b/app/src/main/kotlin/com/wire/android/services/ServicesManager.kt @@ -52,7 +52,7 @@ class ServicesManager @Inject constructor( init { scope.launch { callServiceEvents - .debounce { if (!it) 0L else DEBOUNCE_TIME } // debounce to avoid starting and stopping service too fast + .debounce { if (it) 0L else DEBOUNCE_TIME } // debounce to avoid starting and stopping service too fast .distinctUntilChanged() .collectLatest { shouldBeStarted -> if (!shouldBeStarted) { @@ -127,6 +127,6 @@ class ServicesManager @Inject constructor( companion object { @VisibleForTesting - internal const val DEBOUNCE_TIME = 200L + const val DEBOUNCE_TIME = 500L } } diff --git a/app/src/test/kotlin/com/wire/android/services/ServicesManagerTest.kt b/app/src/test/kotlin/com/wire/android/services/ServicesManagerTest.kt index 8f9d51ec833..6e0684c6c83 100644 --- a/app/src/test/kotlin/com/wire/android/services/ServicesManagerTest.kt +++ b/app/src/test/kotlin/com/wire/android/services/ServicesManagerTest.kt @@ -39,7 +39,7 @@ class ServicesManagerTest { val dispatcherProvider = TestDispatcherProvider() @Test - fun `given ongoing call service running, when stop comes instantly after start, then do not even start the service`() = + fun `given ongoing call service running, when stop comes instantly after start, then start the service and stop it after a while`() = runTest(dispatcherProvider.main()) { // given val (arrangement, servicesManager) = Arrangement() @@ -47,11 +47,27 @@ class ServicesManagerTest { .arrange() // when servicesManager.startCallService() - advanceTimeBy((ServicesManager.DEBOUNCE_TIME - 50).milliseconds) servicesManager.stopCallService() // then - verify(exactly = 0) { arrangement.context.startService(arrangement.ongoingCallServiceIntent) } - verify(exactly = 1) { arrangement.context.stopService(arrangement.ongoingCallServiceIntent) } + verify(exactly = 1) { arrangement.context.startService(arrangement.callServiceIntent) } + advanceUntilIdle() + verify(exactly = 1) { arrangement.context.stopService(arrangement.callServiceIntent) } + } + + @Test + fun `given call service running, when start comes instantly after stop, then do not stop the service`() = + runTest(dispatcherProvider.main()) { + // given + val (arrangement, servicesManager) = Arrangement() + .withServiceState(CallService.ServiceState.FOREGROUND) + .arrange() + servicesManager.startCallService() + // when + servicesManager.stopCallService() + servicesManager.startCallService() + // then + verify(exactly = 1) { arrangement.context.startService(arrangement.callServiceIntent) } + verify(exactly = 0) { arrangement.context.stopService(arrangement.callServiceIntent) } } @Test @@ -67,8 +83,9 @@ class ServicesManagerTest { advanceTimeBy((ServicesManager.DEBOUNCE_TIME + 50).milliseconds) servicesManager.stopCallService() // then - verify(exactly = 1) { arrangement.context.startService(arrangement.ongoingCallServiceIntent) } - verify(exactly = 1) { arrangement.context.stopService(arrangement.ongoingCallServiceIntent) } + verify(exactly = 1) { arrangement.context.startService(arrangement.callServiceIntent) } + advanceUntilIdle() + verify(exactly = 1) { arrangement.context.stopService(arrangement.callServiceIntent) } } @Test @@ -79,13 +96,13 @@ class ServicesManagerTest { .withServiceState(CallService.ServiceState.FOREGROUND) .arrange() servicesManager.startCallService() - advanceUntilIdle() arrangement.clearRecordedCallsForContext() // clear calls recorded when initializing the state // when servicesManager.stopCallService() + advanceUntilIdle() // then verify(exactly = 0) { arrangement.context.startService(arrangement.ongoingCallServiceIntentWithStopArgument) } - verify(exactly = 1) { arrangement.context.stopService(arrangement.ongoingCallServiceIntent) } + verify(exactly = 1) { arrangement.context.stopService(arrangement.callServiceIntent) } } @Test @@ -96,13 +113,13 @@ class ServicesManagerTest { .withServiceState(CallService.ServiceState.STARTED) .arrange() servicesManager.startCallService() - advanceUntilIdle() arrangement.clearRecordedCallsForContext() // clear calls recorded when initializing the state // when servicesManager.stopCallService() + advanceUntilIdle() // then verify(exactly = 1) { arrangement.context.startService(arrangement.ongoingCallServiceIntentWithStopArgument) } - verify(exactly = 0) { arrangement.context.stopService(arrangement.ongoingCallServiceIntent) } + verify(exactly = 0) { arrangement.context.stopService(arrangement.callServiceIntent) } } @Test @@ -119,7 +136,7 @@ class ServicesManagerTest { servicesManager.startCallService() // then verify(exactly = 0) { arrangement.context.startService(arrangement.ongoingCallServiceIntentWithStopArgument) } - verify(exactly = 0) { arrangement.context.stopService(arrangement.ongoingCallServiceIntent) } + verify(exactly = 0) { arrangement.context.stopService(arrangement.callServiceIntent) } } private inner class Arrangement { @@ -130,7 +147,7 @@ class ServicesManagerTest { private val servicesManager: ServicesManager by lazy { ServicesManager(context, dispatcherProvider) } @MockK - lateinit var ongoingCallServiceIntent: Intent + lateinit var callServiceIntent: Intent @MockK lateinit var ongoingCallServiceIntentWithStopArgument: Intent @@ -138,7 +155,7 @@ class ServicesManagerTest { init { MockKAnnotations.init(this, relaxUnitFun = true) mockkObject(CallService.Companion) - every { CallService.Companion.newIntent(context) } returns ongoingCallServiceIntent + every { CallService.Companion.newIntent(context) } returns callServiceIntent every { CallService.Companion.newIntentToStop(context) } returns ongoingCallServiceIntentWithStopArgument }