Skip to content

Commit 300fea0

Browse files
saleniuksaleniuk
authored
fix: do not close SFT 1on1 call when other user audio is not established [WPB-19591] (#3838)
1 parent 8478f6d commit 300fea0

File tree

5 files changed

+125
-154
lines changed

5 files changed

+125
-154
lines changed

‎logic/src/commonJvmAndroid/kotlin/com/wire/kalium/logic/feature/call/CallManagerImpl.kt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -586,8 +586,7 @@ internal class CallManagerImpl internal constructor(
586586
callRepository = callRepository,
587587
qualifiedIdMapper = qualifiedIdMapper,
588588
participantMapper = ParticipantMapperImpl(videoStateChecker, callMapper, qualifiedIdMapper),
589-
userConfigRepository = userConfigRepository,
590-
callHelper = CallHelperImpl(),
589+
callHelper = CallHelperImpl(userConfigRepository, callRepository),
591590
endCall = { endCall(it) },
592591
callingScope = scope
593592
).keepingStrongReference()

‎logic/src/commonJvmAndroid/kotlin/com/wire/kalium/logic/feature/call/scenario/OnParticipantListChanged.kt‎

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,9 @@ package com.wire.kalium.logic.feature.call.scenario
2020

2121
import com.sun.jna.Pointer
2222
import com.wire.kalium.calling.callbacks.ParticipantChangedHandler
23-
import com.wire.kalium.common.functional.getOrElse
2423
import com.wire.kalium.common.logger.callingLogger
2524
import com.wire.kalium.common.logger.kaliumLogger
2625
import com.wire.kalium.logger.obfuscateId
27-
import com.wire.kalium.logic.configuration.UserConfigRepository
2826
import com.wire.kalium.logic.data.call.CallHelper
2927
import com.wire.kalium.logic.data.call.CallParticipants
3028
import com.wire.kalium.logic.data.call.CallRepository
@@ -33,7 +31,6 @@ import com.wire.kalium.logic.data.call.mapper.ParticipantMapper
3331
import com.wire.kalium.logic.data.id.ConversationId
3432
import com.wire.kalium.logic.data.id.QualifiedIdMapper
3533
import kotlinx.coroutines.CoroutineScope
36-
import kotlinx.coroutines.flow.first
3734
import kotlinx.coroutines.launch
3835
import kotlinx.serialization.json.Json
3936

@@ -42,7 +39,6 @@ internal class OnParticipantListChanged internal constructor(
4239
private val callRepository: CallRepository,
4340
private val qualifiedIdMapper: QualifiedIdMapper,
4441
private val participantMapper: ParticipantMapper,
45-
private val userConfigRepository: UserConfigRepository,
4642
private val callHelper: CallHelper,
4743
private val endCall: suspend (conversationId: ConversationId) -> Unit,
4844
private val callingScope: CoroutineScope,
@@ -66,23 +62,13 @@ internal class OnParticipantListChanged internal constructor(
6662
participants.add(participantMapper.fromCallMemberToParticipantMinimized(member))
6763
}
6864

69-
if (userConfigRepository.shouldUseSFTForOneOnOneCalls().getOrElse(false)) {
70-
val callProtocol = callRepository.currentCallProtocol(conversationIdWithDomain)
71-
72-
val currentCall = callRepository.establishedCallsFlow().first().firstOrNull()
73-
currentCall?.let {
74-
val shouldEndSFTOneOnOneCall = callHelper.shouldEndSFTOneOnOneCall(
75-
conversationId = conversationIdWithDomain,
76-
callProtocol = callProtocol,
77-
conversationType = it.conversationType,
78-
newCallParticipants = participants,
79-
previousCallParticipants = it.participants
80-
)
81-
if (shouldEndSFTOneOnOneCall) {
82-
kaliumLogger.i("[onParticipantChanged] - Ending SFT one on one call due to participant leaving")
83-
endCall(conversationIdWithDomain)
84-
}
85-
}
65+
val shouldEndSFTOneOnOneCall = callHelper.shouldEndSFTOneOnOneCall(
66+
conversationId = conversationIdWithDomain,
67+
newCallParticipants = participants
68+
)
69+
if (shouldEndSFTOneOnOneCall) {
70+
kaliumLogger.i("[onParticipantChanged] - Ending SFT one on one call due to participant leaving")
71+
endCall(conversationIdWithDomain)
8672
}
8773

8874
callRepository.updateCallParticipants(

‎logic/src/commonMain/kotlin/com/wire/kalium/logic/data/call/CallHelper.kt‎

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,12 @@
1717
*/
1818
package com.wire.kalium.logic.data.call
1919

20+
import com.wire.kalium.common.functional.getOrElse
21+
import com.wire.kalium.logic.configuration.UserConfigRepository
2022
import com.wire.kalium.logic.data.conversation.Conversation
2123
import com.wire.kalium.logic.data.id.ConversationId
2224
import io.mockative.Mockable
25+
import kotlinx.coroutines.flow.firstOrNull
2326

2427
/**
2528
* Helper class to handle call related operations.
@@ -28,46 +31,39 @@ import io.mockative.Mockable
2831
internal interface CallHelper {
2932

3033
/**
31-
* Check if the OneOnOne call that uses SFT should be ended.
32-
* For Proteus, the call should be ended if the call has one participant after having 2 in the call.
33-
* For MLS, the call should be ended if the call has two participants and the second participant has lost audio.
34+
* Check if the OneOnOne call that uses SFT should be ended when the participants of that call are changed.
35+
* The call should be ended in that case when:
36+
* - the config states that SFT should be used for 1on1 calls
37+
* - the call for given conversationId is established
38+
* - the conversation is 1on1
39+
* - the participants of the call are changed from 2 to 1, for both Proteus and MLS
3440
*
3541
* @param conversationId the conversation id.
36-
* @param callProtocol the call protocol.
37-
* @param conversationType the conversation type.
3842
* @param newCallParticipants the new call participants.
39-
* @param previousCallParticipants the previous call participants.
4043
* @return true if the call should be ended, false otherwise.
4144
*/
42-
fun shouldEndSFTOneOnOneCall(
45+
suspend fun shouldEndSFTOneOnOneCall(
4346
conversationId: ConversationId,
44-
callProtocol: Conversation.ProtocolInfo?,
45-
conversationType: Conversation.Type,
4647
newCallParticipants: List<ParticipantMinimized>,
47-
previousCallParticipants: List<Participant>
4848
): Boolean
4949
}
5050

51-
internal class CallHelperImpl : CallHelper {
51+
internal class CallHelperImpl(
52+
private val userConfigRepository: UserConfigRepository,
53+
private val callRepository: CallRepository,
54+
) : CallHelper {
5255

53-
override fun shouldEndSFTOneOnOneCall(
56+
override suspend fun shouldEndSFTOneOnOneCall(
5457
conversationId: ConversationId,
55-
callProtocol: Conversation.ProtocolInfo?,
56-
conversationType: Conversation.Type,
5758
newCallParticipants: List<ParticipantMinimized>,
58-
previousCallParticipants: List<Participant>
59-
): Boolean {
60-
return if (callProtocol is Conversation.ProtocolInfo.Proteus) {
61-
conversationType == Conversation.Type.OneOnOne &&
62-
newCallParticipants.size == ONE_PARTICIPANTS &&
63-
previousCallParticipants.size == TWO_PARTICIPANTS
64-
} else {
65-
conversationType == Conversation.Type.OneOnOne &&
66-
newCallParticipants.size == TWO_PARTICIPANTS &&
67-
previousCallParticipants.size == TWO_PARTICIPANTS &&
68-
previousCallParticipants[1].hasEstablishedAudio && !newCallParticipants[1].hasEstablishedAudio
69-
}
70-
}
59+
): Boolean =
60+
userConfigRepository.shouldUseSFTForOneOnOneCalls().getOrElse(false).takeIf { it }?.let {
61+
callRepository.establishedCallsFlow().firstOrNull()?.firstOrNull { it.conversationId == conversationId }?.let { call ->
62+
call.conversationType == Conversation.Type.OneOnOne &&
63+
call.participants.size == TWO_PARTICIPANTS &&
64+
newCallParticipants.size == ONE_PARTICIPANTS
65+
}
66+
} ?: false
7167

7268
internal companion object {
7369
internal const val TWO_PARTICIPANTS = 2

‎logic/src/commonTest/kotlin/com/wire/kalium/logic/data/call/CallHelperTest.kt‎

Lines changed: 90 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -18,118 +18,108 @@
1818
package com.wire.kalium.logic.data.call
1919

2020
import com.wire.kalium.common.error.StorageFailure
21+
import com.wire.kalium.common.functional.Either
22+
import com.wire.kalium.common.functional.left
23+
import com.wire.kalium.common.functional.right
2124
import com.wire.kalium.logic.configuration.UserConfigRepository
2225
import com.wire.kalium.logic.data.conversation.Conversation
2326
import com.wire.kalium.logic.data.id.ConversationId
24-
import com.wire.kalium.logic.data.id.GroupID
2527
import com.wire.kalium.logic.data.id.QualifiedID
26-
import com.wire.kalium.logic.data.mls.CipherSuite
27-
import com.wire.kalium.common.functional.Either
28+
import com.wire.kalium.logic.data.user.UserId
2829
import io.mockative.coEvery
29-
import io.mockative.of
30-
import io.mockative.every
3130
import io.mockative.mock
31+
import io.mockative.of
32+
import kotlinx.coroutines.flow.flowOf
3233
import kotlinx.coroutines.test.runTest
33-
import kotlinx.datetime.Instant
3434
import kotlin.test.Test
35-
import kotlin.test.assertFalse
36-
import kotlin.test.assertTrue
35+
import kotlin.test.assertEquals
3736

3837
class CallHelperTest {
3938

39+
private fun testShouldEndSFT1on1Call(
40+
shouldUseSFTForOneOnOneCalls: Either<StorageFailure, Boolean> = true.right(),
41+
establishedCall: Call? = call,
42+
newCallParticipants: List<ParticipantMinimized> = listOf(participantMinimized1),
43+
expected: Boolean
44+
) = runTest {
45+
val (_, callHelper) = Arrangement()
46+
.withShouldUseSFTForOneOnOneCallsReturning(shouldUseSFTForOneOnOneCalls)
47+
.withEstablishedCallsFlowReturning(listOfNotNull(establishedCall))
48+
.arrange()
49+
assertEquals(expected, callHelper.shouldEndSFTOneOnOneCall(conversationId, newCallParticipants))
50+
}
51+
4052
@Test
41-
fun givenMlsProtocol_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnCorrectValue() =
42-
runTest {
43-
val (_, mLSCallHelper) = Arrangement()
44-
.withShouldUseSFTForOneOnOneCallsReturning(Either.Right(true))
45-
.arrange()
46-
47-
// one participant in the call
48-
val shouldEndSFTOneOnOneCall1 = mLSCallHelper.shouldEndSFTOneOnOneCall(
49-
conversationId = conversationId,
50-
callProtocol = CONVERSATION_MLS_PROTOCOL_INFO,
51-
conversationType = Conversation.Type.OneOnOne,
52-
newCallParticipants = listOf(participantMinimized1),
53-
previousCallParticipants = listOf(participant1)
54-
)
55-
assertFalse { shouldEndSFTOneOnOneCall1 }
56-
57-
// Audio not lost for the second participant
58-
val shouldEndSFTOneOnOneCall2 = mLSCallHelper.shouldEndSFTOneOnOneCall(
59-
conversationId = conversationId,
60-
callProtocol = CONVERSATION_MLS_PROTOCOL_INFO,
61-
conversationType = Conversation.Type.Group.Regular,
62-
newCallParticipants = listOf(participantMinimized1, participantMinimized2),
63-
previousCallParticipants = listOf(participant1, participant2)
64-
)
65-
assertFalse { shouldEndSFTOneOnOneCall2 }
66-
67-
// Audio lost for the second participant
68-
val shouldEndSFTOneOnOneCall3 = mLSCallHelper.shouldEndSFTOneOnOneCall(
69-
conversationId = conversationId,
70-
callProtocol = CONVERSATION_MLS_PROTOCOL_INFO,
71-
conversationType = Conversation.Type.OneOnOne,
72-
previousCallParticipants = listOf(participant1, participant2),
73-
newCallParticipants = listOf(
74-
participantMinimized1,
75-
participantMinimized2.copy(hasEstablishedAudio = false)
76-
)
77-
)
78-
assertTrue { shouldEndSFTOneOnOneCall3 }
79-
}
53+
fun givenSFTFor1on1CallsConfigNotFound_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnFalse() =
54+
testShouldEndSFT1on1Call(shouldUseSFTForOneOnOneCalls = StorageFailure.DataNotFound.left(), expected = false)
8055

8156
@Test
82-
fun givenProteusProtocol_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnCorrectValue() =
83-
runTest {
84-
85-
val (_, mLSCallHelper) = Arrangement()
86-
.withShouldUseSFTForOneOnOneCallsReturning(Either.Right(true))
87-
.arrange()
88-
89-
// participants list has 2 items for the new list and the previous list
90-
val shouldEndSFTOneOnOneCall1 = mLSCallHelper.shouldEndSFTOneOnOneCall(
91-
conversationId = conversationId,
92-
callProtocol = Conversation.ProtocolInfo.Proteus,
93-
conversationType = Conversation.Type.OneOnOne,
94-
newCallParticipants = listOf(participantMinimized1, participantMinimized2),
95-
previousCallParticipants = listOf(participant1, participant2)
96-
)
97-
assertFalse { shouldEndSFTOneOnOneCall1 }
98-
99-
// new participants list has 1 participant
100-
val shouldEndSFTOneOnOneCall2 = mLSCallHelper.shouldEndSFTOneOnOneCall(
101-
conversationId = conversationId,
102-
callProtocol = Conversation.ProtocolInfo.Proteus,
103-
conversationType = Conversation.Type.OneOnOne,
104-
newCallParticipants = listOf(participantMinimized1),
105-
previousCallParticipants = listOf(participant1, participant2)
106-
)
107-
assertTrue { shouldEndSFTOneOnOneCall2 }
108-
}
57+
fun givenSFTShouldNotBeUsedFor1on1Calls_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnFalse() =
58+
testShouldEndSFT1on1Call(shouldUseSFTForOneOnOneCalls = false.right(), expected = false)
59+
60+
@Test
61+
fun givenNotEstablishedCall_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnFalse() =
62+
testShouldEndSFT1on1Call(establishedCall = null, expected = false)
63+
64+
@Test
65+
fun givenEstablishedNon1on1Call_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnFalse() =
66+
testShouldEndSFT1on1Call(establishedCall = call.copy(conversationType = Conversation.Type.Group.Regular), expected = false)
67+
68+
@Test
69+
fun givenEstablished1on1CallWith1Participant_andParticipantsDidNotChange_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnFalse() =
70+
testShouldEndSFT1on1Call(
71+
establishedCall = call.copy(participants = listOf(participant1)),
72+
newCallParticipants = listOf(participantMinimized1),
73+
expected = false
74+
)
75+
76+
@Test
77+
fun givenEstablished1on1CallWith2Participants_andParticipantsDidNotChange_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnFalse() =
78+
testShouldEndSFT1on1Call(
79+
establishedCall = call.copy(participants = listOf(participant1, participant2)),
80+
newCallParticipants = listOf(participantMinimized1, participantMinimized2),
81+
expected = false
82+
)
83+
84+
@Test
85+
fun givenEstablished1on1CallWith1Participant_andOneParticipantJoined_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnFalse() =
86+
testShouldEndSFT1on1Call(
87+
establishedCall = call.copy(participants = listOf(participant1)),
88+
newCallParticipants = listOf(participantMinimized1, participantMinimized2),
89+
expected = false
90+
)
91+
92+
@Test
93+
fun givenEstablished1on1CallWith2Participants_andOneParticipantLeft_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnTrue() =
94+
testShouldEndSFT1on1Call(
95+
establishedCall = call.copy(participants = listOf(participant1, participant2)),
96+
newCallParticipants = listOf(participantMinimized1),
97+
expected = true
98+
)
10999

110100
private class Arrangement {
111101

112102
val userConfigRepository = mock(of<UserConfigRepository>())
113-
114-
private val mLSCallHelper: CallHelper = CallHelperImpl()
103+
val callRepository = mock(of<CallRepository>())
104+
private val mLSCallHelper: CallHelper = CallHelperImpl(userConfigRepository, callRepository)
115105

116106
fun arrange() = this to mLSCallHelper
117107

118-
suspend fun withShouldUseSFTForOneOnOneCallsReturning(result: Either<StorageFailure, Boolean>) =
119-
apply {
120-
coEvery { userConfigRepository.shouldUseSFTForOneOnOneCalls() }.returns(result)
121-
}
108+
suspend fun withShouldUseSFTForOneOnOneCallsReturning(result: Either<StorageFailure, Boolean>) = apply {
109+
coEvery {
110+
userConfigRepository.shouldUseSFTForOneOnOneCalls()
111+
}.returns(result)
112+
}
113+
114+
suspend fun withEstablishedCallsFlowReturning(calls: List<Call>) = apply {
115+
coEvery {
116+
callRepository.establishedCallsFlow()
117+
}.returns(flowOf(calls))
118+
}
122119
}
123120

124121
companion object {
125122
val conversationId = ConversationId(value = "convId", domain = "domainId")
126-
val CONVERSATION_MLS_PROTOCOL_INFO = Conversation.ProtocolInfo.MLS(
127-
GroupID("GROUP_ID"),
128-
Conversation.ProtocolInfo.MLSCapable.GroupState.ESTABLISHED,
129-
5UL,
130-
Instant.parse("2021-03-30T15:36:00.000Z"),
131-
cipherSuite = CipherSuite.MLS_128_DHKEMX25519_AES128GCM_SHA256_Ed25519
132-
)
133123
val participant1 = Participant(
134124
id = QualifiedID("participantId", "participantDomain"),
135125
clientId = "abcd",
@@ -159,5 +149,18 @@ class CallHelperTest {
159149
id = QualifiedID("participantId2", "participantDomain2"),
160150
clientId = "efgh"
161151
)
152+
val call = Call(
153+
conversationId = conversationId,
154+
status = CallStatus.ESTABLISHED,
155+
isMuted = true,
156+
isCameraOn = false,
157+
isCbrEnabled = false,
158+
callerId = UserId("callerId", "domain"),
159+
conversationName = "Conversation Name",
160+
conversationType = Conversation.Type.OneOnOne,
161+
callerName = "name",
162+
callerTeamName = "team",
163+
participants = listOf(participant1, participant2)
164+
)
162165
}
163166
}

0 commit comments

Comments
 (0)