Skip to content

fix: do not close SFT 1on1 call when other user audio is not established [WPB-19591] #3838

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
saleniuk wants to merge 1 commit into
base: develop
Choose a base branch
from
+125 −154
Open

fix: do not close SFT 1on1 call when other user audio is not established [WPB-19591] #3838

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions logic/src/commonJvmAndroid/kotlin/com/wire/kalium/logic/feature/call/CallManagerImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
28 changes: 7 additions & 21 deletions ...JvmAndroid/kotlin/com/wire/kalium/logic/feature/call/scenario/OnParticipantListChanged.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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,
Expand All @@ -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(
Expand Down
50 changes: 23 additions & 27 deletions logic/src/commonMain/kotlin/com/wire/kalium/logic/data/call/CallHelper.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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<ParticipantMinimized>,
previousCallParticipants: List<Participant>
): 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<ParticipantMinimized>,
previousCallParticipants: List<Participant>
): 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
Expand Down
177 changes: 90 additions & 87 deletions logic/src/commonTest/kotlin/com/wire/kalium/logic/data/call/CallHelperTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<StorageFailure, Boolean> = true.right(),
establishedCall: Call? = call,
newCallParticipants: List<ParticipantMinimized> = 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<UserConfigRepository>())

private val mLSCallHelper: CallHelper = CallHelperImpl()
val callRepository = mock(of<CallRepository>())
private val mLSCallHelper: CallHelper = CallHelperImpl(userConfigRepository, callRepository)

fun arrange() = this to mLSCallHelper

suspend fun withShouldUseSFTForOneOnOneCallsReturning(result: Either<StorageFailure, Boolean>) =
apply {
coEvery { userConfigRepository.shouldUseSFTForOneOnOneCalls() }.returns(result)
}
suspend fun withShouldUseSFTForOneOnOneCallsReturning(result: Either<StorageFailure, Boolean>) = apply {
coEvery {
userConfigRepository.shouldUseSFTForOneOnOneCalls()
}.returns(result)
}

suspend fun withEstablishedCallsFlowReturning(calls: List<Call>) = 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",
Expand Down Expand Up @@ -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)
)
}
}
Loading
Loading