From 3a09719dac8f8daf35d05e35312dd4246ce9f882 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= <30429749+saleniuk@users.noreply.github.com> Date: Mon, 20 Jan 2025 10:23:05 +0100 Subject: [PATCH] fix: answer call from notification in foreground service [WPB-9648] (#3797) --- .../IncomingCallActionReceiver.kt | 9 +- .../com/wire/android/services/CallService.kt | 58 ++++++++--- .../wire/android/services/ServicesManager.kt | 29 ++++-- .../android/services/ServicesManagerTest.kt | 98 ++++++++++++++++++- 4 files changed, 167 insertions(+), 27 deletions(-) diff --git a/app/src/main/kotlin/com/wire/android/notification/broadcastreceivers/IncomingCallActionReceiver.kt b/app/src/main/kotlin/com/wire/android/notification/broadcastreceivers/IncomingCallActionReceiver.kt index 66936e76f11..4c7ca4cea9c 100644 --- a/app/src/main/kotlin/com/wire/android/notification/broadcastreceivers/IncomingCallActionReceiver.kt +++ b/app/src/main/kotlin/com/wire/android/notification/broadcastreceivers/IncomingCallActionReceiver.kt @@ -26,6 +26,7 @@ import com.wire.android.di.ApplicationScope import com.wire.android.di.KaliumCoreLogic import com.wire.android.di.NoSession import com.wire.android.notification.CallNotificationManager +import com.wire.android.services.ServicesManager import com.wire.android.util.dispatchers.DispatcherProvider import com.wire.kalium.logger.obfuscateId import com.wire.kalium.logic.CoreLogic @@ -59,6 +60,9 @@ class IncomingCallActionReceiver : BroadcastReceiver() { @Inject lateinit var callNotificationManager: CallNotificationManager + @Inject + lateinit var servicesManager: ServicesManager + @Suppress("ReturnCount") override fun onReceive(context: Context, intent: Intent) { val conversationIdString: String = intent.getStringExtra(EXTRA_CONVERSATION_ID) ?: run { @@ -77,9 +81,10 @@ class IncomingCallActionReceiver : BroadcastReceiver() { coroutineScope.launch(Dispatchers.Default) { with(coreLogic.getSessionScope(userId)) { + val conversationId = qualifiedIdMapper.fromStringToQualifiedID(conversationIdString) when (action) { - ACTION_DECLINE_CALL -> calls.rejectCall(qualifiedIdMapper.fromStringToQualifiedID(conversationIdString)) - ACTION_ANSWER_CALL -> calls.answerCall(qualifiedIdMapper.fromStringToQualifiedID(conversationIdString)) + ACTION_DECLINE_CALL -> calls.rejectCall(conversationId) + ACTION_ANSWER_CALL -> servicesManager.startCallServiceToAnswer(userId, conversationId) } } callNotificationManager.hideIncomingCallNotification(userId.toString(), conversationIdString) 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 27c75804b16..c8172b624db 100644 --- a/app/src/main/kotlin/com/wire/android/services/CallService.kt +++ b/app/src/main/kotlin/com/wire/android/services/CallService.kt @@ -31,15 +31,20 @@ import com.wire.android.di.NoSession import com.wire.android.notification.CallNotificationData import com.wire.android.notification.CallNotificationManager import com.wire.android.notification.NotificationIds +import com.wire.android.services.CallService.Action import com.wire.android.util.dispatchers.DispatcherProvider import com.wire.android.util.logIfEmptyUserName import com.wire.kalium.logic.CoreLogic import com.wire.kalium.logic.data.call.CallStatus +import com.wire.kalium.logic.data.id.ConversationId import com.wire.kalium.logic.data.id.QualifiedIdMapper +import com.wire.kalium.logic.data.user.UserId +import com.wire.kalium.logic.feature.call.CallsScope import com.wire.kalium.logic.feature.session.CurrentSessionResult import com.wire.kalium.logic.functional.Either import com.wire.kalium.logic.functional.fold import dagger.hilt.android.AndroidEntryPoint +import dev.ahmedmourad.bundlizer.Bundlizer import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancel @@ -50,7 +55,9 @@ import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch +import kotlinx.serialization.Serializable import java.util.concurrent.atomic.AtomicReference import javax.inject.Inject @@ -90,14 +97,17 @@ class CallService : Service() { override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int { appLogger.i("$TAG: onStartCommand") - val stopService = intent?.getBooleanExtra(EXTRA_STOP_SERVICE, false) + val action = intent?.getActionTypeExtra(EXTRA_ACTION_TYPE) generatePlaceholderForegroundNotification() serviceState.set(ServiceState.FOREGROUND) - if (stopService == true) { + if (action is Action.Stop) { appLogger.i("$TAG: stopSelf. Reason: stopService was called") stopSelf() } else { scope.launch { + if (action is Action.AnswerCall) { + coreLogic.getSessionScope(action.userId).calls.answerCall(action.conversationId) + } coreLogic.getGlobalScope().session.currentSessionFlow() .flatMapLatest { if (it is CurrentSessionResult.Success && it.accountInfo.isValid()) { @@ -105,12 +115,14 @@ class CallService : Service() { val userSessionScope = coreLogic.getSessionScope(userId) val outgoingCallsFlow = userSessionScope.calls.observeOutgoingCall() val establishedCallsFlow = userSessionScope.calls.establishedCall() + val callCurrentlyBeingAnsweredFlow = userSessionScope.calls.observeCallCurrentlyBeingAnswered(action) combine( outgoingCallsFlow, - establishedCallsFlow - ) { outgoingCalls, establishedCalls -> - val calls = outgoingCalls + establishedCalls + establishedCallsFlow, + callCurrentlyBeingAnsweredFlow + ) { outgoingCalls, establishedCalls, answeringCall -> + val calls = outgoingCalls + establishedCalls + answeringCall calls.firstOrNull()?.let { call -> val userName = userSessionScope.users.observeSelfUser().first() .also { it.logIfEmptyUserName() } @@ -180,22 +192,40 @@ class CallService : Service() { appLogger.i("$TAG: started foreground with placeholder notification") } + private suspend fun CallsScope.observeCallCurrentlyBeingAnswered(action: Action?) = when (action) { + is Action.AnswerCall -> getIncomingCalls().map { it.filter { it.conversationId == action.conversationId } } + else -> flowOf(emptyList()) + } + companion object { private const val TAG = "CallService" - private const val EXTRA_STOP_SERVICE = "stop_service" - - fun newIntent(context: Context): Intent = Intent(context, CallService::class.java) + private const val EXTRA_ACTION_TYPE = "action_type" - fun newIntentToStop(context: Context): Intent = - Intent(context, CallService::class.java).apply { - putExtra(EXTRA_STOP_SERVICE, true) - } + fun newIntent(context: Context, actionType: Action = Action.Default): Intent = Intent(context, CallService::class.java) + .putExtra(EXTRA_ACTION_TYPE, actionType) - var serviceState: AtomicReference = AtomicReference(ServiceState.NOT_STARTED) - private set + val serviceState: AtomicReference = AtomicReference(ServiceState.NOT_STARTED) } enum class ServiceState { NOT_STARTED, STARTED, FOREGROUND } + + @Serializable + sealed class Action { + @Serializable + data object Default : Action() + + @Serializable + data class AnswerCall(val userId: UserId, val conversationId: ConversationId) : Action() + + @Serializable + data object Stop : Action() + } } + +private fun Intent.putExtra(name: String, actionType: Action): Intent = putExtra(name, Bundlizer.bundle(Action.serializer(), actionType)) + +private fun Intent.getActionTypeExtra(name: String): Action? = getBundleExtra(name)?.let { + Bundlizer.unbundle(Action.serializer(), it) + } 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 48e00aaf977..aa4d365be68 100644 --- a/app/src/main/kotlin/com/wire/android/services/ServicesManager.kt +++ b/app/src/main/kotlin/com/wire/android/services/ServicesManager.kt @@ -23,9 +23,11 @@ import android.content.Intent import android.os.Build import com.wire.android.appLogger import com.wire.android.util.dispatchers.DispatcherProvider +import com.wire.kalium.logic.data.id.ConversationId +import com.wire.kalium.logic.data.user.UserId import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.distinctUntilChanged @@ -45,15 +47,17 @@ class ServicesManager @Inject constructor( dispatcherProvider: DispatcherProvider, ) { private val scope = CoroutineScope(SupervisorJob() + dispatcherProvider.default()) - private val callServiceEvents = MutableStateFlow(false) + private val callServiceEvents = MutableSharedFlow() init { scope.launch { callServiceEvents - .debounce { if (it) 0L else DEBOUNCE_TIME } // debounce to avoid starting and stopping service too fast + .debounce { action -> + if (action is CallService.Action.Stop) DEBOUNCE_TIME else 0 // debounce to avoid stopping and starting service too fast + } .distinctUntilChanged() - .collectLatest { shouldBeStarted -> - if (!shouldBeStarted) { + .collectLatest { action -> + if (action is CallService.Action.Stop) { appLogger.i("ServicesManager: stopping CallService because there are no calls") when (CallService.serviceState.get()) { CallService.ServiceState.STARTED -> { @@ -62,7 +66,7 @@ class ServicesManager @Inject constructor( // or some specific argument that tells the service that it should stop itself right after startForeground. // This way, when this service is killed and recreated by the system, it will stop itself right after // recreating so it won't cause any problems. - startService(CallService.newIntentToStop(context)) + startService(CallService.newIntent(context, CallService.Action.Stop)) appLogger.i("ServicesManager: CallService stopped by passing stop argument") } @@ -78,7 +82,7 @@ class ServicesManager @Inject constructor( } } else { appLogger.i("ServicesManager: starting CallService") - startService(CallService.newIntent(context)) + startService(CallService.newIntent(context, action)) } } } @@ -87,14 +91,21 @@ class ServicesManager @Inject constructor( fun startCallService() { appLogger.i("ServicesManager: start CallService event") scope.launch { - callServiceEvents.emit(true) + callServiceEvents.emit(CallService.Action.Default) + } + } + + fun startCallServiceToAnswer(userId: UserId, conversationId: ConversationId) { + appLogger.i("ServicesManager: start CallService event") + scope.launch { + callServiceEvents.emit(CallService.Action.AnswerCall(userId, conversationId)) } } fun stopCallService() { appLogger.i("ServicesManager: stop CallService event") scope.launch { - callServiceEvents.emit(false) + callServiceEvents.emit(CallService.Action.Stop) } } 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 6e0684c6c83..cd0ff331e55 100644 --- a/app/src/test/kotlin/com/wire/android/services/ServicesManagerTest.kt +++ b/app/src/test/kotlin/com/wire/android/services/ServicesManagerTest.kt @@ -20,10 +20,13 @@ package com.wire.android.services import android.content.Context import android.content.Intent import com.wire.android.config.TestDispatcherProvider +import com.wire.kalium.logic.data.id.ConversationId +import com.wire.kalium.logic.data.user.UserId import io.mockk.MockKAnnotations import io.mockk.clearMocks import io.mockk.every import io.mockk.impl.annotations.MockK +import io.mockk.mockk import io.mockk.mockkObject import io.mockk.verify import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -139,6 +142,93 @@ class ServicesManagerTest { verify(exactly = 0) { arrangement.context.stopService(arrangement.callServiceIntent) } } + @Test + fun `given ongoing call service running, when start called again with the same action, then do not start the service again`() = + runTest(dispatcherProvider.main()) { + // given + val (arrangement, servicesManager) = Arrangement() + .withServiceState(CallService.ServiceState.FOREGROUND) + .arrange() + servicesManager.startCallService() + advanceUntilIdle() + arrangement.clearRecordedCallsForContext() // clear calls recorded when initializing the state + // when + servicesManager.startCallService() // start again with the same action + advanceUntilIdle() + // then + verify(exactly = 0) { arrangement.context.startService(arrangement.callServiceIntent) } // not called again + } + + @Test + fun `given ongoing call service not started, when answering call, then start with proper action`() = + runTest(dispatcherProvider.main()) { + // given + val userId = UserId("userId", "domain") + val conversationId = ConversationId("conversationId", "domain") + val action = CallService.Action.AnswerCall(userId, conversationId) + val intentForAction = mockk() + val (arrangement, servicesManager) = Arrangement() + .withServiceState(CallService.ServiceState.NOT_STARTED) + .callServiceIntentForAction(action, intentForAction) + .arrange() + advanceUntilIdle() + // when + servicesManager.startCallServiceToAnswer(userId, conversationId) + // then + verify(exactly = 1) { arrangement.context.startService(intentForAction) } + verify(exactly = 0) { arrangement.context.startService(arrangement.callServiceIntent) } + verify(exactly = 0) { arrangement.context.startService(arrangement.ongoingCallServiceIntentWithStopArgument) } + verify(exactly = 0) { arrangement.context.stopService(arrangement.callServiceIntent) } + } + + @Test + fun `given ongoing call service already started, when answering different call, then start again with proper action`() = + runTest(dispatcherProvider.main()) { + // given + val userId = UserId("userId", "domain") + val conversationId = ConversationId("conversationId", "domain") + val action = CallService.Action.AnswerCall(userId, conversationId) + val intentForAction = mockk() + val (arrangement, servicesManager) = Arrangement() + .withServiceState(CallService.ServiceState.NOT_STARTED) + .callServiceIntentForAction(action, intentForAction) + .arrange() + servicesManager.startCallService() + advanceUntilIdle() + arrangement.clearRecordedCallsForContext() // clear calls recorded when initializing the state + // when + servicesManager.startCallServiceToAnswer(userId, conversationId) + // then + verify(exactly = 1) { arrangement.context.startService(intentForAction) } + verify(exactly = 0) { arrangement.context.startService(arrangement.callServiceIntent) } + verify(exactly = 0) { arrangement.context.startService(arrangement.ongoingCallServiceIntentWithStopArgument) } + verify(exactly = 0) { arrangement.context.stopService(arrangement.callServiceIntent) } + } + + @Test + fun `given ongoing call service already started, when needs to answer the same call, then do not start again`() = + runTest(dispatcherProvider.main()) { + // given + val userId = UserId("userId", "domain") + val conversationId = ConversationId("conversationId", "domain") + val action = CallService.Action.AnswerCall(userId, conversationId) + val intentForAction = mockk() + val (arrangement, servicesManager) = Arrangement() + .withServiceState(CallService.ServiceState.NOT_STARTED) + .callServiceIntentForAction(action, intentForAction) + .arrange() + servicesManager.startCallServiceToAnswer(userId, conversationId) + advanceUntilIdle() + arrangement.clearRecordedCallsForContext() // clear calls recorded when initializing the state + // when + servicesManager.startCallServiceToAnswer(userId, conversationId) + // then + verify(exactly = 0) { arrangement.context.startService(intentForAction) } + verify(exactly = 0) { arrangement.context.startService(arrangement.callServiceIntent) } + verify(exactly = 0) { arrangement.context.startService(arrangement.ongoingCallServiceIntentWithStopArgument) } + verify(exactly = 0) { arrangement.context.stopService(arrangement.callServiceIntent) } + } + private inner class Arrangement { @MockK(relaxed = true) @@ -155,8 +245,8 @@ class ServicesManagerTest { init { MockKAnnotations.init(this, relaxUnitFun = true) mockkObject(CallService.Companion) - every { CallService.Companion.newIntent(context) } returns callServiceIntent - every { CallService.Companion.newIntentToStop(context) } returns ongoingCallServiceIntentWithStopArgument + callServiceIntentForAction(CallService.Action.Default, callServiceIntent) + callServiceIntentForAction(CallService.Action.Stop, ongoingCallServiceIntentWithStopArgument) } fun clearRecordedCallsForContext() { @@ -175,6 +265,10 @@ class ServicesManagerTest { every { CallService.serviceState.get() } returns state } + fun callServiceIntentForAction(action: CallService.Action, intent: Intent) = apply { + every { CallService.Companion.newIntent(context, action) } returns intent + } + fun arrange() = this to servicesManager } }