Skip to content

Commit

Permalink
chore: Remove call to deleteSubConversation after ending 1:1 call (WP…
Browse files Browse the repository at this point in the history 
…B-11007) (#3000)

* chore: Remove call to deleteSubConversation after ending 1:1 call

* chore: rename class name

* chore: cleanup

* chore: cleanup

* chore: cleanup

* chore: cleanup
  • Loading branch information
@ohassine
ohassine authored Sep 16, 2024
1 parent 9eafc77 commit 8728f13
Show file tree
Hide file tree
Showing 5 changed files with 236 additions and 372 deletions.
13 changes: 2 additions & 11 deletions logic/src/commonJvmAndroid/kotlin/com/wire/kalium/logic/feature/call/CallManagerImpl.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ import com.wire.kalium.logic.callingLogger
import com.wire.kalium.logic.configuration.UserConfigRepository
import com.wire.kalium.logic.data.call.CallClient
import com.wire.kalium.logic.data.call.CallClientList
import com.wire.kalium.logic.data.call.CallHelperImpl
import com.wire.kalium.logic.data.call.CallRepository
import com.wire.kalium.logic.data.call.CallStatus
import com.wire.kalium.logic.data.call.CallType
import com.wire.kalium.logic.data.call.EpochInfo
import com.wire.kalium.logic.data.call.CallHelperImpl
import com.wire.kalium.logic.data.call.VideoState
import com.wire.kalium.logic.data.call.VideoStateChecker
import com.wire.kalium.logic.data.call.mapper.CallMapper
Expand Down Expand Up @@ -198,11 +198,6 @@ class CallManagerImpl internal constructor(
.keepingStrongReference(),
closeCallHandler = OnCloseCall(
callRepository = callRepository,
callHelper = CallHelperImpl(
callRepository = callRepository,
subconversationRepository = subconversationRepository,
userConfigRepository = userConfigRepository
),
scope = scope,
qualifiedIdMapper = qualifiedIdMapper
).keepingStrongReference(),
Expand Down Expand Up @@ -478,11 +473,7 @@ class CallManagerImpl internal constructor(
participantMapper = ParticipantMapperImpl(videoStateChecker, callMapper),
userRepository = userRepository,
userConfigRepository = userConfigRepository,
callHelper = CallHelperImpl(
callRepository = callRepository,
subconversationRepository = subconversationRepository,
userConfigRepository = userConfigRepository
),
callHelper = CallHelperImpl(),
endCall = { endCall(it) },
callingScope = scope
).keepingStrongReference()
Expand Down
8 changes: 2 additions & 6 deletions logic/src/commonJvmAndroid/kotlin/com/wire/kalium/logic/feature/call/scenario/OnCloseCall.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import com.wire.kalium.logger.obfuscateId
import com.wire.kalium.logic.callingLogger
import com.wire.kalium.logic.data.call.CallRepository
import com.wire.kalium.logic.data.call.CallStatus
import com.wire.kalium.logic.data.call.CallHelper
import com.wire.kalium.logic.data.conversation.Conversation
import com.wire.kalium.logic.data.id.ConversationId
import com.wire.kalium.logic.data.id.QualifiedIdMapper
Expand All @@ -36,7 +35,6 @@ import kotlinx.coroutines.launch
@Suppress("LongParameterList")
class OnCloseCall(
private val callRepository: CallRepository,
private val callHelper: CallHelper,
private val scope: CoroutineScope,
private val qualifiedIdMapper: QualifiedIdMapper
) : CloseCallHandler {
Expand Down Expand Up @@ -69,12 +67,10 @@ class OnCloseCall(
status = callStatus
)

val conversationType =
callRepository.getCallMetadataProfile()[conversationIdWithDomain]?.conversationType

if (callRepository.getCallMetadataProfile()[conversationIdWithDomain]?.protocol is Conversation.ProtocolInfo.MLS) {
callHelper.handleCallTermination(conversationIdWithDomain, conversationType)
callRepository.leaveMlsConference(conversationIdWithDomain)
}

callingLogger.i("[OnCloseCall] -> ConversationId: ${conversationId.obfuscateId()} | callStatus: $callStatus")
}
}
Expand Down
59 changes: 1 addition & 58 deletions logic/src/commonMain/kotlin/com/wire/kalium/logic/data/call/CallHelper.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,8 @@
*/
package com.wire.kalium.logic.data.call

import com.wire.kalium.logic.callingLogger
import com.wire.kalium.logic.configuration.UserConfigRepository
import com.wire.kalium.logic.data.conversation.Conversation
import com.wire.kalium.logic.data.conversation.SubconversationRepository
import com.wire.kalium.logic.data.id.ConversationId
import com.wire.kalium.logic.data.id.SubconversationId
import com.wire.kalium.logic.data.id.toModel
import com.wire.kalium.logic.functional.getOrElse
import com.wire.kalium.logic.functional.onFailure
import com.wire.kalium.logic.functional.onSuccess
import com.wire.kalium.network.api.base.authenticated.conversation.SubconversationDeleteRequest

/**
* Helper class to handle call related operations.
Expand All @@ -53,27 +44,9 @@ interface CallHelper {
newCallParticipants: List<Participant>,
previousCallParticipants: List<Participant>
): Boolean

/**
* Handle the call termination.
* If the call is oneOneOne on SFT, then delete MLS sub conversation
* otherwise leave the MLS conference.
*
* @param conversationId the conversation id.
* @param callProtocol the call protocol.
* @param conversationType the conversation type.
*/
suspend fun handleCallTermination(
conversationId: ConversationId,
conversationType: Conversation.Type?
)
}

class CallHelperImpl(
private val callRepository: CallRepository,
private val subconversationRepository: SubconversationRepository,
private val userConfigRepository: UserConfigRepository
) : CallHelper {
class CallHelperImpl : CallHelper {

override fun shouldEndSFTOneOnOneCall(
conversationId: ConversationId,
Expand All @@ -94,36 +67,6 @@ class CallHelperImpl(
}
}

override suspend fun handleCallTermination(
conversationId: ConversationId,
conversationType: Conversation.Type?
) {
if (userConfigRepository.shouldUseSFTForOneOnOneCalls().getOrElse(false) &&
conversationType == Conversation.Type.ONE_ON_ONE
) {
callingLogger.i("[CallHelper] -> fetching remote MLS sub conversation details")
subconversationRepository.fetchRemoteSubConversationDetails(
conversationId,
CALL_SUBCONVERSATION_ID
).onSuccess { subconversationDetails ->
callingLogger.i("[CallHelper] -> Deleting remote MLS sub conversation")
subconversationRepository.deleteRemoteSubConversation(
subconversationDetails.parentId.toModel(),
SubconversationId(subconversationDetails.id),
SubconversationDeleteRequest(
subconversationDetails.epoch,
subconversationDetails.groupId
)
)
}.onFailure {
callingLogger.e("[CallHelper] -> Error fetching remote MLS sub conversation details")
}
} else {
callingLogger.i("[CallHelper] -> Leaving MLS conference")
callRepository.leaveMlsConference(conversationId)
}
}

companion object {
const val TWO_PARTICIPANTS = 2
const val ONE_PARTICIPANTS = 1
Expand Down
120 changes: 2 additions & 118 deletions ...lium/logic/data/call/MLSCallHelperTest.kt → .../kalium/logic/data/call/CallHelperTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
*/
package com.wire.kalium.logic.data.call

import com.wire.kalium.logic.NetworkFailure
import com.wire.kalium.logic.StorageFailure
import com.wire.kalium.logic.configuration.UserConfigRepository
import com.wire.kalium.logic.data.conversation.Conversation
Expand All @@ -27,23 +26,17 @@ 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.logic.functional.Either
import com.wire.kalium.logic.test_util.TestNetworkException
import com.wire.kalium.network.api.base.authenticated.conversation.SubconversationResponse
import io.mockative.Mock
import io.mockative.any
import io.mockative.classOf
import io.mockative.eq
import io.mockative.given
import io.mockative.mock
import io.mockative.once
import io.mockative.verify
import kotlinx.coroutines.test.runTest
import kotlinx.datetime.Instant
import kotlin.test.Test
import kotlin.test.assertFalse
import kotlin.test.assertTrue

class MLSCallHelperTest {
class CallHelperTest {

@Test
fun givenMlsProtocol_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnCorrectValue() =
Expand Down Expand Up @@ -115,93 +108,12 @@ class MLSCallHelperTest {
assertTrue { shouldEndSFTOneOnOneCall2 }
}

@Test
fun givenMLSOneOnOneCallAndShouldUseSFTForOneOnOneCall_whenHandleCallTerminationIsCalled_thenDeleteRemoteSubConversation() =
runTest {
val (arrangement, mLSCallHelper) = Arrangement()
.withShouldUseSFTForOneOnOneCallsReturning(Either.Right(true))
.withFetchRemoteSubConversationDetailsReturning(
Either.Right(subconversationResponse)
)
.withDeleteRemoteSubConversationSuccess()
.arrange()

mLSCallHelper.handleCallTermination(conversationId, Conversation.Type.ONE_ON_ONE)

verify(arrangement.subconversationRepository)
.suspendFunction(arrangement.subconversationRepository::deleteRemoteSubConversation)
.with(any(), any(), any())
.wasInvoked(exactly = once)
}

@Test
fun givenSubconversationRepositoryReturnFailure_whenHandleCallTerminationIsCalled_thenDoNotDeleteRemoteSubConversation() =
runTest {
val (arrangement, mLSCallHelper) = Arrangement()
.withShouldUseSFTForOneOnOneCallsReturning(Either.Right(true))
.withFetchRemoteSubConversationDetailsReturning(
Either.Left(
NetworkFailure.ServerMiscommunication(
TestNetworkException.badRequest
)
)
)
.arrange()

mLSCallHelper.handleCallTermination(conversationId, Conversation.Type.ONE_ON_ONE)

verify(arrangement.subconversationRepository)
.suspendFunction(arrangement.subconversationRepository::deleteRemoteSubConversation)
.with(eq(conversationId))
.wasNotInvoked()
}

@Test
fun givenShouldNotUseSFTForOneOnOneCall_whenHandleCallTerminationIsCalled_thenLeaveMlsConference() =
runTest {
val (arrangement, mLSCallHelper) = Arrangement()
.withShouldUseSFTForOneOnOneCallsReturning(Either.Right(false))
.arrange()

mLSCallHelper.handleCallTermination(conversationId, Conversation.Type.GROUP)

verify(arrangement.callRepository)
.suspendFunction(arrangement.callRepository::leaveMlsConference)
.with(eq(conversationId))
.wasInvoked(exactly = once)
}

@Test
fun givenMLSGroupCall_whenHandleCallTerminationIsCalled_thenLeaveMlsConference() =
runTest {
val (arrangement, mLSCallHelper) = Arrangement()
.withShouldUseSFTForOneOnOneCallsReturning(Either.Right(true))
.arrange()

mLSCallHelper.handleCallTermination(conversationId, Conversation.Type.GROUP)

verify(arrangement.callRepository)
.suspendFunction(arrangement.callRepository::leaveMlsConference)
.with(eq(conversationId))
.wasInvoked(exactly = once)
}

private class Arrangement {

@Mock
val callRepository = mock(classOf<CallRepository>())

@Mock
val subconversationRepository = mock(classOf<SubconversationRepository>())

@Mock
val userConfigRepository = mock(classOf<UserConfigRepository>())

private val mLSCallHelper: CallHelper = CallHelperImpl(
callRepository = callRepository,
subconversationRepository = subconversationRepository,
userConfigRepository = userConfigRepository
)
private val mLSCallHelper: CallHelper = CallHelperImpl()

fun arrange() = this to mLSCallHelper

Expand All @@ -212,22 +124,6 @@ class MLSCallHelperTest {
.whenInvoked()
.thenReturn(result)
}

fun withFetchRemoteSubConversationDetailsReturning(result: Either<NetworkFailure, SubconversationResponse>) =
apply {
given(subconversationRepository)
.suspendFunction(subconversationRepository::fetchRemoteSubConversationDetails)
.whenInvokedWith(eq(conversationId), eq(CALL_SUBCONVERSATION_ID))
.thenReturn(result)
}

fun withDeleteRemoteSubConversationSuccess() =
apply {
given(subconversationRepository)
.suspendFunction(subconversationRepository::deleteRemoteSubConversation)
.whenInvokedWith(any(), any(), any())
.thenReturn(Either.Right(Unit))
}
}

companion object {
Expand All @@ -254,17 +150,5 @@ class MLSCallHelperTest {
id = QualifiedID("participantId2", "participantDomain2"),
clientId = "efgh"
)
val subconversationResponse = SubconversationResponse(
id = "subconversationId",
parentId = com.wire.kalium.network.api.base.model.ConversationId(
"conversationId",
"domainId"
),
groupId = "groupId",
epoch = 1UL,
epochTimestamp = "2021-03-30T15:36:00.000Z",
mlsCipherSuiteTag = 5,
members = listOf()
)
}
}
Loading

0 comments on commit 8728f13

Please sign in to comment.