Skip to content
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.

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

chore: Remove call to deleteSubConversation after ending 1:1 call (WPB-11007) #3000

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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
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
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
Loading