diff --git a/logic/src/commonJvmAndroid/kotlin/com/wire/kalium/logic/feature/call/CallManagerImpl.kt b/logic/src/commonJvmAndroid/kotlin/com/wire/kalium/logic/feature/call/CallManagerImpl.kt index 1366de3d207..cc78568a836 100644 --- a/logic/src/commonJvmAndroid/kotlin/com/wire/kalium/logic/feature/call/CallManagerImpl.kt +++ b/logic/src/commonJvmAndroid/kotlin/com/wire/kalium/logic/feature/call/CallManagerImpl.kt @@ -586,8 +586,7 @@ internal class CallManagerImpl internal constructor( callRepository = callRepository, qualifiedIdMapper = qualifiedIdMapper, participantMapper = ParticipantMapperImpl(videoStateChecker, callMapper, qualifiedIdMapper), - userConfigRepository = userConfigRepository, - callHelper = CallHelperImpl(), + callHelper = CallHelperImpl(userConfigRepository, callRepository), endCall = { endCall(it) }, callingScope = scope ).keepingStrongReference() diff --git a/logic/src/commonJvmAndroid/kotlin/com/wire/kalium/logic/feature/call/scenario/OnParticipantListChanged.kt b/logic/src/commonJvmAndroid/kotlin/com/wire/kalium/logic/feature/call/scenario/OnParticipantListChanged.kt index e8de30954fe..f0a0b002ac3 100644 --- a/logic/src/commonJvmAndroid/kotlin/com/wire/kalium/logic/feature/call/scenario/OnParticipantListChanged.kt +++ b/logic/src/commonJvmAndroid/kotlin/com/wire/kalium/logic/feature/call/scenario/OnParticipantListChanged.kt @@ -20,11 +20,9 @@ package com.wire.kalium.logic.feature.call.scenario import com.sun.jna.Pointer import com.wire.kalium.calling.callbacks.ParticipantChangedHandler -import com.wire.kalium.common.functional.getOrElse import com.wire.kalium.common.logger.callingLogger import com.wire.kalium.common.logger.kaliumLogger import com.wire.kalium.logger.obfuscateId -import com.wire.kalium.logic.configuration.UserConfigRepository import com.wire.kalium.logic.data.call.CallHelper import com.wire.kalium.logic.data.call.CallParticipants import com.wire.kalium.logic.data.call.CallRepository @@ -33,7 +31,6 @@ import com.wire.kalium.logic.data.call.mapper.ParticipantMapper import com.wire.kalium.logic.data.id.ConversationId import com.wire.kalium.logic.data.id.QualifiedIdMapper import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.serialization.json.Json @@ -42,7 +39,6 @@ internal class OnParticipantListChanged internal constructor( private val callRepository: CallRepository, private val qualifiedIdMapper: QualifiedIdMapper, private val participantMapper: ParticipantMapper, - private val userConfigRepository: UserConfigRepository, private val callHelper: CallHelper, private val endCall: suspend (conversationId: ConversationId) -> Unit, private val callingScope: CoroutineScope, @@ -66,23 +62,13 @@ internal class OnParticipantListChanged internal constructor( participants.add(participantMapper.fromCallMemberToParticipantMinimized(member)) } - if (userConfigRepository.shouldUseSFTForOneOnOneCalls().getOrElse(false)) { - val callProtocol = callRepository.currentCallProtocol(conversationIdWithDomain) - - val currentCall = callRepository.establishedCallsFlow().first().firstOrNull() - currentCall?.let { - val shouldEndSFTOneOnOneCall = callHelper.shouldEndSFTOneOnOneCall( - conversationId = conversationIdWithDomain, - callProtocol = callProtocol, - conversationType = it.conversationType, - newCallParticipants = participants, - previousCallParticipants = it.participants - ) - if (shouldEndSFTOneOnOneCall) { - kaliumLogger.i("[onParticipantChanged] - Ending SFT one on one call due to participant leaving") - endCall(conversationIdWithDomain) - } - } + val shouldEndSFTOneOnOneCall = callHelper.shouldEndSFTOneOnOneCall( + conversationId = conversationIdWithDomain, + newCallParticipants = participants + ) + if (shouldEndSFTOneOnOneCall) { + kaliumLogger.i("[onParticipantChanged] - Ending SFT one on one call due to participant leaving") + endCall(conversationIdWithDomain) } callRepository.updateCallParticipants( diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/call/CallHelper.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/call/CallHelper.kt index 2806063756f..b5e84424f11 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/call/CallHelper.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/call/CallHelper.kt @@ -17,9 +17,12 @@ */ package com.wire.kalium.logic.data.call +import com.wire.kalium.common.functional.getOrElse +import com.wire.kalium.logic.configuration.UserConfigRepository import com.wire.kalium.logic.data.conversation.Conversation import com.wire.kalium.logic.data.id.ConversationId import io.mockative.Mockable +import kotlinx.coroutines.flow.firstOrNull /** * Helper class to handle call related operations. @@ -28,46 +31,39 @@ import io.mockative.Mockable internal interface CallHelper { /** - * Check if the OneOnOne call that uses SFT should be ended. - * For Proteus, the call should be ended if the call has one participant after having 2 in the call. - * For MLS, the call should be ended if the call has two participants and the second participant has lost audio. + * Check if the OneOnOne call that uses SFT should be ended when the participants of that call are changed. + * The call should be ended in that case when: + * - the config states that SFT should be used for 1on1 calls + * - the call for given conversationId is established + * - the conversation is 1on1 + * - the participants of the call are changed from 2 to 1, for both Proteus and MLS * * @param conversationId the conversation id. - * @param callProtocol the call protocol. - * @param conversationType the conversation type. * @param newCallParticipants the new call participants. - * @param previousCallParticipants the previous call participants. * @return true if the call should be ended, false otherwise. */ - fun shouldEndSFTOneOnOneCall( + suspend fun shouldEndSFTOneOnOneCall( conversationId: ConversationId, - callProtocol: Conversation.ProtocolInfo?, - conversationType: Conversation.Type, newCallParticipants: List, - previousCallParticipants: List ): Boolean } -internal class CallHelperImpl : CallHelper { +internal class CallHelperImpl( + private val userConfigRepository: UserConfigRepository, + private val callRepository: CallRepository, +) : CallHelper { - override fun shouldEndSFTOneOnOneCall( + override suspend fun shouldEndSFTOneOnOneCall( conversationId: ConversationId, - callProtocol: Conversation.ProtocolInfo?, - conversationType: Conversation.Type, newCallParticipants: List, - previousCallParticipants: List - ): Boolean { - return if (callProtocol is Conversation.ProtocolInfo.Proteus) { - conversationType == Conversation.Type.OneOnOne && - newCallParticipants.size == ONE_PARTICIPANTS && - previousCallParticipants.size == TWO_PARTICIPANTS - } else { - conversationType == Conversation.Type.OneOnOne && - newCallParticipants.size == TWO_PARTICIPANTS && - previousCallParticipants.size == TWO_PARTICIPANTS && - previousCallParticipants[1].hasEstablishedAudio && !newCallParticipants[1].hasEstablishedAudio - } - } + ): Boolean = + userConfigRepository.shouldUseSFTForOneOnOneCalls().getOrElse(false).takeIf { it }?.let { + callRepository.establishedCallsFlow().firstOrNull()?.firstOrNull { it.conversationId == conversationId }?.let { call -> + call.conversationType == Conversation.Type.OneOnOne && + call.participants.size == TWO_PARTICIPANTS && + newCallParticipants.size == ONE_PARTICIPANTS + } + } ?: false internal companion object { internal const val TWO_PARTICIPANTS = 2 diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/call/CallHelperTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/call/CallHelperTest.kt index 7df701fbe43..2a1f805c537 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/call/CallHelperTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/call/CallHelperTest.kt @@ -18,118 +18,108 @@ package com.wire.kalium.logic.data.call import com.wire.kalium.common.error.StorageFailure +import com.wire.kalium.common.functional.Either +import com.wire.kalium.common.functional.left +import com.wire.kalium.common.functional.right import com.wire.kalium.logic.configuration.UserConfigRepository import com.wire.kalium.logic.data.conversation.Conversation import com.wire.kalium.logic.data.id.ConversationId -import com.wire.kalium.logic.data.id.GroupID import com.wire.kalium.logic.data.id.QualifiedID -import com.wire.kalium.logic.data.mls.CipherSuite -import com.wire.kalium.common.functional.Either +import com.wire.kalium.logic.data.user.UserId import io.mockative.coEvery -import io.mockative.of -import io.mockative.every import io.mockative.mock +import io.mockative.of +import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runTest -import kotlinx.datetime.Instant import kotlin.test.Test -import kotlin.test.assertFalse -import kotlin.test.assertTrue +import kotlin.test.assertEquals class CallHelperTest { + private fun testShouldEndSFT1on1Call( + shouldUseSFTForOneOnOneCalls: Either = true.right(), + establishedCall: Call? = call, + newCallParticipants: List = listOf(participantMinimized1), + expected: Boolean + ) = runTest { + val (_, callHelper) = Arrangement() + .withShouldUseSFTForOneOnOneCallsReturning(shouldUseSFTForOneOnOneCalls) + .withEstablishedCallsFlowReturning(listOfNotNull(establishedCall)) + .arrange() + assertEquals(expected, callHelper.shouldEndSFTOneOnOneCall(conversationId, newCallParticipants)) + } + @Test - fun givenMlsProtocol_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnCorrectValue() = - runTest { - val (_, mLSCallHelper) = Arrangement() - .withShouldUseSFTForOneOnOneCallsReturning(Either.Right(true)) - .arrange() - - // one participant in the call - val shouldEndSFTOneOnOneCall1 = mLSCallHelper.shouldEndSFTOneOnOneCall( - conversationId = conversationId, - callProtocol = CONVERSATION_MLS_PROTOCOL_INFO, - conversationType = Conversation.Type.OneOnOne, - newCallParticipants = listOf(participantMinimized1), - previousCallParticipants = listOf(participant1) - ) - assertFalse { shouldEndSFTOneOnOneCall1 } - - // Audio not lost for the second participant - val shouldEndSFTOneOnOneCall2 = mLSCallHelper.shouldEndSFTOneOnOneCall( - conversationId = conversationId, - callProtocol = CONVERSATION_MLS_PROTOCOL_INFO, - conversationType = Conversation.Type.Group.Regular, - newCallParticipants = listOf(participantMinimized1, participantMinimized2), - previousCallParticipants = listOf(participant1, participant2) - ) - assertFalse { shouldEndSFTOneOnOneCall2 } - - // Audio lost for the second participant - val shouldEndSFTOneOnOneCall3 = mLSCallHelper.shouldEndSFTOneOnOneCall( - conversationId = conversationId, - callProtocol = CONVERSATION_MLS_PROTOCOL_INFO, - conversationType = Conversation.Type.OneOnOne, - previousCallParticipants = listOf(participant1, participant2), - newCallParticipants = listOf( - participantMinimized1, - participantMinimized2.copy(hasEstablishedAudio = false) - ) - ) - assertTrue { shouldEndSFTOneOnOneCall3 } - } + fun givenSFTFor1on1CallsConfigNotFound_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnFalse() = + testShouldEndSFT1on1Call(shouldUseSFTForOneOnOneCalls = StorageFailure.DataNotFound.left(), expected = false) @Test - fun givenProteusProtocol_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnCorrectValue() = - runTest { - - val (_, mLSCallHelper) = Arrangement() - .withShouldUseSFTForOneOnOneCallsReturning(Either.Right(true)) - .arrange() - - // participants list has 2 items for the new list and the previous list - val shouldEndSFTOneOnOneCall1 = mLSCallHelper.shouldEndSFTOneOnOneCall( - conversationId = conversationId, - callProtocol = Conversation.ProtocolInfo.Proteus, - conversationType = Conversation.Type.OneOnOne, - newCallParticipants = listOf(participantMinimized1, participantMinimized2), - previousCallParticipants = listOf(participant1, participant2) - ) - assertFalse { shouldEndSFTOneOnOneCall1 } - - // new participants list has 1 participant - val shouldEndSFTOneOnOneCall2 = mLSCallHelper.shouldEndSFTOneOnOneCall( - conversationId = conversationId, - callProtocol = Conversation.ProtocolInfo.Proteus, - conversationType = Conversation.Type.OneOnOne, - newCallParticipants = listOf(participantMinimized1), - previousCallParticipants = listOf(participant1, participant2) - ) - assertTrue { shouldEndSFTOneOnOneCall2 } - } + fun givenSFTShouldNotBeUsedFor1on1Calls_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnFalse() = + testShouldEndSFT1on1Call(shouldUseSFTForOneOnOneCalls = false.right(), expected = false) + + @Test + fun givenNotEstablishedCall_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnFalse() = + testShouldEndSFT1on1Call(establishedCall = null, expected = false) + + @Test + fun givenEstablishedNon1on1Call_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnFalse() = + testShouldEndSFT1on1Call(establishedCall = call.copy(conversationType = Conversation.Type.Group.Regular), expected = false) + + @Test + fun givenEstablished1on1CallWith1Participant_andParticipantsDidNotChange_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnFalse() = + testShouldEndSFT1on1Call( + establishedCall = call.copy(participants = listOf(participant1)), + newCallParticipants = listOf(participantMinimized1), + expected = false + ) + + @Test + fun givenEstablished1on1CallWith2Participants_andParticipantsDidNotChange_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnFalse() = + testShouldEndSFT1on1Call( + establishedCall = call.copy(participants = listOf(participant1, participant2)), + newCallParticipants = listOf(participantMinimized1, participantMinimized2), + expected = false + ) + + @Test + fun givenEstablished1on1CallWith1Participant_andOneParticipantJoined_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnFalse() = + testShouldEndSFT1on1Call( + establishedCall = call.copy(participants = listOf(participant1)), + newCallParticipants = listOf(participantMinimized1, participantMinimized2), + expected = false + ) + + @Test + fun givenEstablished1on1CallWith2Participants_andOneParticipantLeft_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnTrue() = + testShouldEndSFT1on1Call( + establishedCall = call.copy(participants = listOf(participant1, participant2)), + newCallParticipants = listOf(participantMinimized1), + expected = true + ) private class Arrangement { val userConfigRepository = mock(of()) - - private val mLSCallHelper: CallHelper = CallHelperImpl() + val callRepository = mock(of()) + private val mLSCallHelper: CallHelper = CallHelperImpl(userConfigRepository, callRepository) fun arrange() = this to mLSCallHelper - suspend fun withShouldUseSFTForOneOnOneCallsReturning(result: Either) = - apply { - coEvery { userConfigRepository.shouldUseSFTForOneOnOneCalls() }.returns(result) - } + suspend fun withShouldUseSFTForOneOnOneCallsReturning(result: Either) = apply { + coEvery { + userConfigRepository.shouldUseSFTForOneOnOneCalls() + }.returns(result) + } + + suspend fun withEstablishedCallsFlowReturning(calls: List) = apply { + coEvery { + callRepository.establishedCallsFlow() + }.returns(flowOf(calls)) + } } companion object { val conversationId = ConversationId(value = "convId", domain = "domainId") - val CONVERSATION_MLS_PROTOCOL_INFO = Conversation.ProtocolInfo.MLS( - GroupID("GROUP_ID"), - Conversation.ProtocolInfo.MLSCapable.GroupState.ESTABLISHED, - 5UL, - Instant.parse("2021-03-30T15:36:00.000Z"), - cipherSuite = CipherSuite.MLS_128_DHKEMX25519_AES128GCM_SHA256_Ed25519 - ) val participant1 = Participant( id = QualifiedID("participantId", "participantDomain"), clientId = "abcd", @@ -159,5 +149,18 @@ class CallHelperTest { id = QualifiedID("participantId2", "participantDomain2"), clientId = "efgh" ) + val call = Call( + conversationId = conversationId, + status = CallStatus.ESTABLISHED, + isMuted = true, + isCameraOn = false, + isCbrEnabled = false, + callerId = UserId("callerId", "domain"), + conversationName = "Conversation Name", + conversationType = Conversation.Type.OneOnOne, + callerName = "name", + callerTeamName = "team", + participants = listOf(participant1, participant2) + ) } } diff --git a/logic/src/jvmTest/kotlin/com/wire/kalium/logic/feature/call/scenario/OnParticipantListChangedTest.kt b/logic/src/jvmTest/kotlin/com/wire/kalium/logic/feature/call/scenario/OnParticipantListChangedTest.kt index 48d2da6c446..ed22ae9b369 100644 --- a/logic/src/jvmTest/kotlin/com/wire/kalium/logic/feature/call/scenario/OnParticipantListChangedTest.kt +++ b/logic/src/jvmTest/kotlin/com/wire/kalium/logic/feature/call/scenario/OnParticipantListChangedTest.kt @@ -17,9 +17,6 @@ */ package com.wire.kalium.logic.feature.call.scenario -import com.wire.kalium.common.error.StorageFailure -import com.wire.kalium.common.functional.Either -import com.wire.kalium.logic.configuration.UserConfigRepository import com.wire.kalium.logic.data.call.Call import com.wire.kalium.logic.data.call.CallHelper import com.wire.kalium.logic.data.call.CallRepository @@ -67,7 +64,7 @@ class OnParticipantListChangedTest { testScope.runTest { val (arrangement, onParticipantListChanged) = Arrangement() .withParticipantMapper() - .withUserConfigRepositoryReturning(Either.Left(StorageFailure.DataNotFound)) + .withShouldEndSFTOneOnOneCall(false) .arrange() onParticipantListChanged.onParticipantChanged( @@ -89,7 +86,6 @@ class OnParticipantListChangedTest { testScope.runTest { val (arrangement, onParticipantListChanged) = Arrangement() .withParticipantMapper() - .withUserConfigRepositoryReturning(Either.Right(true)) .withProtocol() .withEstablishedCall() .withShouldEndSFTOneOnOneCall(true) @@ -110,7 +106,6 @@ class OnParticipantListChangedTest { testScope.runTest { val (arrangement, onParticipantListChanged) = Arrangement() .withParticipantMapper() - .withUserConfigRepositoryReturning(Either.Right(true)) .withProtocol() .withEstablishedCall() .withShouldEndSFTOneOnOneCall(false) @@ -128,7 +123,6 @@ class OnParticipantListChangedTest { internal class Arrangement { val callRepository = mock(CallRepository::class) val participantMapper = mock(ParticipantMapper::class) - val userConfigRepository = mock(UserConfigRepository::class) val callHelper = mock(CallHelper::class) var isEndCallInvoked = false @@ -138,7 +132,6 @@ class OnParticipantListChangedTest { fun arrange() = this to OnParticipantListChanged( callRepository = callRepository, participantMapper = participantMapper, - userConfigRepository = userConfigRepository, callHelper = callHelper, qualifiedIdMapper = qualifiedIdMapper, endCall = { @@ -147,12 +140,6 @@ class OnParticipantListChangedTest { callingScope = testScope, ) - suspend fun withUserConfigRepositoryReturning(result: Either) = apply { - coEvery { - userConfigRepository.shouldUseSFTForOneOnOneCalls() - }.returns(result) - } - fun withParticipantMapper() = apply { every { participantMapper.fromCallMemberToParticipantMinimized(any()) @@ -171,9 +158,9 @@ class OnParticipantListChangedTest { }.returns(flowOf(listOf(call))) } - fun withShouldEndSFTOneOnOneCall(result: Boolean) = apply { - every { - callHelper.shouldEndSFTOneOnOneCall(any(), any(), any(), any(), any()) + suspend fun withShouldEndSFTOneOnOneCall(result: Boolean) = apply { + coEvery { + callHelper.shouldEndSFTOneOnOneCall(any(), any()) }.returns(result) } }