From 58d7ee8954351aa34bf9ceec054bfdfc7e9ba96e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 13 Aug 2024 13:11:19 +0200 Subject: [PATCH] =?UTF-8?q?feat:=20End=20SFT=20oneOnOne=20call=20on=20prot?= =?UTF-8?q?eus=20protocol=20(WPB-7153)=20=F0=9F=8D=92=20(#2936)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Commit with unresolved merge conflicts * chore: resolve conflicts --------- Co-authored-by: Oussama Hassine --- .../logic/feature/call/CallManagerImpl.kt | 1 + .../call/scenario/OnParticipantListChanged.kt | 38 ++- .../kalium/logic/data/call/MLSCallHelper.kt | 17 +- .../logic/data/call/MLSCallHelperTest.kt | 83 +++--- .../scenario/OnParticipantListChangedTest.kt | 276 ++++++++++++++++++ 5 files changed, 349 insertions(+), 66 deletions(-) create mode 100644 logic/src/jvmTest/kotlin/com/wire/kalium/logic/feature/call/scenario/OnParticipantListChangedTest.kt 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 3d67e47c3be..1b04710a4a0 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 @@ -542,6 +542,7 @@ class CallManagerImpl internal constructor( qualifiedIdMapper = qualifiedIdMapper, participantMapper = ParticipantMapperImpl(videoStateChecker, callMapper), userRepository = userRepository, + userConfigRepository = userConfigRepository, mlsCallHelper = MLSCallHelperImpl( callRepository = callRepository, subconversationRepository = subconversationRepository, 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 98933839485..18995e095bb 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 @@ -22,6 +22,7 @@ import com.sun.jna.Pointer import com.wire.kalium.calling.callbacks.ParticipantChangedHandler import com.wire.kalium.logger.obfuscateId import com.wire.kalium.logic.callingLogger +import com.wire.kalium.logic.configuration.UserConfigRepository import com.wire.kalium.logic.data.call.CallParticipants import com.wire.kalium.logic.data.call.CallRepository import com.wire.kalium.logic.data.call.MLSCallHelper @@ -30,6 +31,7 @@ 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 com.wire.kalium.logic.data.user.UserRepository +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.logic.kaliumLogger @@ -38,21 +40,22 @@ import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.serialization.json.Json -// TODO: add tests for this class @Suppress("LongParameterList") class OnParticipantListChanged internal constructor( private val callRepository: CallRepository, private val qualifiedIdMapper: QualifiedIdMapper, private val participantMapper: ParticipantMapper, private val userRepository: UserRepository, + private val userConfigRepository: UserConfigRepository, private val mlsCallHelper: MLSCallHelper, private val endCall: suspend (conversationId: ConversationId) -> Unit, - private val callingScope: CoroutineScope + private val callingScope: CoroutineScope, + private val jsonDecoder: Json = Json ) : ParticipantChangedHandler { override fun onParticipantChanged(remoteConversationId: String, data: String, arg: Pointer?) { - val participantsChange = Json.decodeFromString(data) + val participantsChange = jsonDecoder.decodeFromString(data) callingScope.launch { val participants = mutableListOf() @@ -72,20 +75,23 @@ class OnParticipantListChanged internal constructor( participants.add(participant) } } - val callProtocol = callRepository.currentCallProtocol(conversationIdWithDomain) - val currentCall = callRepository.establishedCallsFlow().first().firstOrNull() - currentCall?.let { - val shouldEndSFTOneOnOneCall = mlsCallHelper.shouldEndSFTOneOnOneCall( - conversationId = conversationIdWithDomain, - callProtocol = callProtocol, - conversationType = it.conversationType, - newCallParticipants = participants, - previousCallParticipants = it.participants - ) - if (shouldEndSFTOneOnOneCall) { - kaliumLogger.i("[onParticipantChanged] - Ending MLS call due to participant leaving") - endCall(conversationIdWithDomain) + if (userConfigRepository.shouldUseSFTForOneOnOneCalls().getOrElse(false)) { + val callProtocol = callRepository.currentCallProtocol(conversationIdWithDomain) + + val currentCall = callRepository.establishedCallsFlow().first().firstOrNull() + currentCall?.let { + val shouldEndSFTOneOnOneCall = mlsCallHelper.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) + } } } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/call/MLSCallHelper.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/call/MLSCallHelper.kt index 24ed1b131cb..9a87fee9c3d 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/call/MLSCallHelper.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/call/MLSCallHelper.kt @@ -80,12 +80,18 @@ class MLSCallHelperImpl( conversationType: Conversation.Type, newCallParticipants: List, previousCallParticipants: List - ) = callProtocol is Conversation.ProtocolInfo.MLS && - userConfigRepository.shouldUseSFTForOneOnOneCalls().getOrElse(false) && + ): Boolean { + return if (callProtocol is Conversation.ProtocolInfo.Proteus) { conversationType == Conversation.Type.ONE_ON_ONE && - newCallParticipants.size == TWO_PARTICIPANTS && - previousCallParticipants.size == TWO_PARTICIPANTS && - previousCallParticipants[1].hasEstablishedAudio && !newCallParticipants[1].hasEstablishedAudio + newCallParticipants.size == ONE_PARTICIPANTS && + previousCallParticipants.size == TWO_PARTICIPANTS + } else { + conversationType == Conversation.Type.ONE_ON_ONE && + newCallParticipants.size == TWO_PARTICIPANTS && + previousCallParticipants.size == TWO_PARTICIPANTS && + previousCallParticipants[1].hasEstablishedAudio && !newCallParticipants[1].hasEstablishedAudio + } + } override suspend fun handleCallTermination( conversationId: ConversationId, @@ -119,5 +125,6 @@ class MLSCallHelperImpl( companion object { const val TWO_PARTICIPANTS = 2 + const val ONE_PARTICIPANTS = 1 } } diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/call/MLSCallHelperTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/call/MLSCallHelperTest.kt index 1c23dd67698..5482eceb708 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/call/MLSCallHelperTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/call/MLSCallHelperTest.kt @@ -47,44 +47,34 @@ import kotlin.test.assertTrue class MLSCallHelperTest { @Test - fun givenMlsConferenceCall_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnCorrectValue() = + fun givenMlsProtocol_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnCorrectValue() = runTest { val (_, mLSCallHelper) = Arrangement() .withShouldUseSFTForOneOnOneCallsReturning(Either.Right(true)) .arrange() - // Proteus protocol - val shouldEndSFTOneOnOneCall1 = mLSCallHelper.shouldEndSFTOneOnOneCall( - conversationId = conversationId, - callProtocol = Conversation.ProtocolInfo.Proteus, - conversationType = Conversation.Type.ONE_ON_ONE, - newCallParticipants = listOf(participant1, participant2), - previousCallParticipants = listOf(participant1, participant2) - ) - assertFalse { shouldEndSFTOneOnOneCall1 } - // one participant in the call - val shouldEndSFTOneOnOneCall2 = mLSCallHelper.shouldEndSFTOneOnOneCall( + val shouldEndSFTOneOnOneCall1 = mLSCallHelper.shouldEndSFTOneOnOneCall( conversationId = conversationId, callProtocol = CONVERSATION_MLS_PROTOCOL_INFO, conversationType = Conversation.Type.ONE_ON_ONE, newCallParticipants = listOf(participant1), previousCallParticipants = listOf(participant1) ) - assertFalse { shouldEndSFTOneOnOneCall2 } + assertFalse { shouldEndSFTOneOnOneCall1 } - // Audi not lost for the second participant - val shouldEndSFTOneOnOneCall3 = mLSCallHelper.shouldEndSFTOneOnOneCall( + // Audio not lost for the second participant + val shouldEndSFTOneOnOneCall2 = mLSCallHelper.shouldEndSFTOneOnOneCall( conversationId = conversationId, callProtocol = CONVERSATION_MLS_PROTOCOL_INFO, conversationType = Conversation.Type.GROUP, newCallParticipants = listOf(participant1, participant2), previousCallParticipants = listOf(participant1, participant2) ) - assertFalse { shouldEndSFTOneOnOneCall3 } + assertFalse { shouldEndSFTOneOnOneCall2 } // Audio lost for the second participant - val shouldEndSFTOneOnOneCall4 = mLSCallHelper.shouldEndSFTOneOnOneCall( + val shouldEndSFTOneOnOneCall3 = mLSCallHelper.shouldEndSFTOneOnOneCall( conversationId = conversationId, callProtocol = CONVERSATION_MLS_PROTOCOL_INFO, conversationType = Conversation.Type.ONE_ON_ONE, @@ -94,24 +84,36 @@ class MLSCallHelperTest { participant2.copy(hasEstablishedAudio = false) ) ) - assertTrue { shouldEndSFTOneOnOneCall4 } + assertTrue { shouldEndSFTOneOnOneCall3 } + } - val (_, mLSCallHelper2) = Arrangement() - .withShouldUseSFTForOneOnOneCallsReturning(Either.Left(StorageFailure.DataNotFound)) + @Test + fun givenProteusProtocol_whenShouldEndSFTOneOnOneCallIsCalled_thenReturnCorrectValue() = + runTest { + + val (_, mLSCallHelper) = Arrangement() + .withShouldUseSFTForOneOnOneCallsReturning(Either.Right(true)) .arrange() - // ShouldUseSFTForOneOnOneCalls user config is not found - val shouldEndSFTOneOnOneCall5 = mLSCallHelper2.shouldEndSFTOneOnOneCall( + // participants list has 2 items for the new list and the previous list + val shouldEndSFTOneOnOneCall1 = mLSCallHelper.shouldEndSFTOneOnOneCall( conversationId = conversationId, - callProtocol = CONVERSATION_MLS_PROTOCOL_INFO, + callProtocol = Conversation.ProtocolInfo.Proteus, conversationType = Conversation.Type.ONE_ON_ONE, - previousCallParticipants = listOf(participant1, participant2), - newCallParticipants = listOf( - participant1, - participant2.copy(hasEstablishedAudio = false) - ) + newCallParticipants = listOf(participant1, participant2), + previousCallParticipants = listOf(participant1, participant2) ) - assertFalse { shouldEndSFTOneOnOneCall5 } + assertFalse { shouldEndSFTOneOnOneCall1 } + + // new participants list has 1 participant + val shouldEndSFTOneOnOneCall2 = mLSCallHelper.shouldEndSFTOneOnOneCall( + conversationId = conversationId, + callProtocol = Conversation.ProtocolInfo.Proteus, + conversationType = Conversation.Type.ONE_ON_ONE, + newCallParticipants = listOf(participant1), + previousCallParticipants = listOf(participant1, participant2) + ) + assertTrue { shouldEndSFTOneOnOneCall2 } } @Test @@ -129,7 +131,7 @@ class MLSCallHelperTest { coVerify { arrangement.subconversationRepository.deleteRemoteSubConversation(any(), any(), any()) - }.wasInvoked(once) + }.wasInvoked(exactly = once) } @Test @@ -149,11 +151,7 @@ class MLSCallHelperTest { mLSCallHelper.handleCallTermination(conversationId, Conversation.Type.ONE_ON_ONE) coVerify { - arrangement.subconversationRepository.deleteRemoteSubConversation( - eq(conversationId), - any(), - any() - ) + arrangement.subconversationRepository.deleteRemoteSubConversation(any(), any(), any()) }.wasNotInvoked() } @@ -167,8 +165,8 @@ class MLSCallHelperTest { mLSCallHelper.handleCallTermination(conversationId, Conversation.Type.GROUP) coVerify { - arrangement.callRepository.leaveMlsConference(eq(conversationId)) - }.wasInvoked(once) + arrangement.callRepository.leaveMlsConference(any()) + }.wasInvoked(exactly = once) } @Test @@ -182,7 +180,7 @@ class MLSCallHelperTest { coVerify { arrangement.callRepository.leaveMlsConference(eq(conversationId)) - }.wasInvoked(once) + }.wasInvoked(exactly = once) } private class Arrangement { @@ -206,18 +204,13 @@ class MLSCallHelperTest { fun withShouldUseSFTForOneOnOneCallsReturning(result: Either) = apply { - every { - userConfigRepository.shouldUseSFTForOneOnOneCalls() - }.returns(result) + every { userConfigRepository.shouldUseSFTForOneOnOneCalls() }.returns(result) } suspend fun withFetchRemoteSubConversationDetailsReturning(result: Either) = apply { coEvery { - subconversationRepository.fetchRemoteSubConversationDetails( - conversationId, - CALL_SUBCONVERSATION_ID - ) + subconversationRepository.fetchRemoteSubConversationDetails(eq(conversationId), eq(CALL_SUBCONVERSATION_ID)) }.returns(result) } 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 new file mode 100644 index 00000000000..659484d8955 --- /dev/null +++ b/logic/src/jvmTest/kotlin/com/wire/kalium/logic/feature/call/scenario/OnParticipantListChangedTest.kt @@ -0,0 +1,276 @@ +/* + * Wire + * Copyright (C) 2024 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.kalium.logic.feature.call.scenario + +import com.wire.kalium.logic.StorageFailure +import com.wire.kalium.logic.configuration.UserConfigRepository +import com.wire.kalium.logic.data.call.Call +import com.wire.kalium.logic.data.call.CallRepository +import com.wire.kalium.logic.data.call.CallStatus +import com.wire.kalium.logic.data.call.MLSCallHelper +import com.wire.kalium.logic.data.call.Participant +import com.wire.kalium.logic.data.call.mapper.ParticipantMapper +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.id.QualifiedIdMapperImpl +import com.wire.kalium.logic.data.mls.CipherSuite +import com.wire.kalium.logic.data.user.OtherUserMinimized +import com.wire.kalium.logic.data.user.UserRepository +import com.wire.kalium.logic.data.user.type.UserType +import com.wire.kalium.logic.framework.TestUser +import com.wire.kalium.logic.functional.Either +import io.mockative.Mock +import io.mockative.any +import io.mockative.coEvery +import io.mockative.coVerify +import io.mockative.every +import io.mockative.mock +import io.mockative.once +import io.mockative.twice +import io.mockative.verify +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.yield +import kotlinx.datetime.Clock +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +@OptIn(ExperimentalCoroutinesApi::class) +class OnParticipantListChangedTest { + + @BeforeTest + fun setup() { + testScope = TestScope() + } + + @Test + fun givenCallRepository_whenParticipantListChangedCallBackHappens_thenUpdateCallParticipantsOnce() = + testScope.runTest { + val (arrangement, onParticipantListChanged) = Arrangement() + .withParticipantMapper() + .withUserRepositorySuccess() + .withUserConfigRepositoryReturning(Either.Left(StorageFailure.DataNotFound)) + .arrange() + + onParticipantListChanged.onParticipantChanged( + REMOTE_CONVERSATION_ID, data, null + ) + yield() + + verify { + arrangement.participantMapper.fromCallMemberToParticipant(any()) + }.wasInvoked(exactly = twice) + + coVerify { + arrangement.userRepository.getKnownUserMinimized(any()) + }.wasInvoked(exactly = twice) + + coVerify { + arrangement.callRepository.updateCallParticipants(any(), any()) + }.wasInvoked(exactly = once) + } + + @Test + fun givenMlsCallHelperReturnsTrue_whenParticipantListChangedCallBackHappens_thenEndCall() = + testScope.runTest { + val (arrangement, onParticipantListChanged) = Arrangement() + .withParticipantMapper() + .withUserRepositorySuccess() + .withUserConfigRepositoryReturning(Either.Right(true)) + .withProtocol() + .withEstablishedCall() + .withShouldEndSFTOneOnOneCall(true) + .arrange() + + onParticipantListChanged.onParticipantChanged( + REMOTE_CONVERSATION_ID, data, null + ) + advanceUntilIdle() + yield() + + assertTrue { arrangement.isEndCallInvoked } + } + + + @Test + fun givenMlsCallHelperReturnsFalse_whenParticipantListChangedCallBackHappens_thenDoNotEndCall() = + testScope.runTest { + val (arrangement, onParticipantListChanged) = Arrangement() + .withParticipantMapper() + .withUserRepositorySuccess() + .withUserConfigRepositoryReturning(Either.Right(true)) + .withProtocol() + .withEstablishedCall() + .withShouldEndSFTOneOnOneCall(false) + .arrange() + + onParticipantListChanged.onParticipantChanged( + REMOTE_CONVERSATION_ID, data, null + ) + yield() + + assertFalse { arrangement.isEndCallInvoked } + } + + + internal class Arrangement { + + @Mock + val callRepository = mock(CallRepository::class) + + @Mock + val participantMapper = mock(ParticipantMapper::class) + + @Mock + val userConfigRepository = mock(UserConfigRepository::class) + + @Mock + val userRepository = mock(UserRepository::class) + + @Mock + val mlsCallHelper = mock(MLSCallHelper::class) + + var isEndCallInvoked = false + + private val qualifiedIdMapper = QualifiedIdMapperImpl(TestUser.SELF.id) + + fun arrange() = this to OnParticipantListChanged( + callRepository = callRepository, + participantMapper = participantMapper, + userConfigRepository = userConfigRepository, + userRepository = userRepository, + mlsCallHelper = mlsCallHelper, + qualifiedIdMapper = qualifiedIdMapper, + endCall = { + isEndCallInvoked = true + }, + callingScope = testScope, + ) + + fun withUserConfigRepositoryReturning(result: Either) = apply { + every { + userConfigRepository.shouldUseSFTForOneOnOneCalls() + }.returns(result) + } + + fun withParticipantMapper() = apply { + every { + participantMapper.fromCallMemberToParticipant(any()) + }.returns(participant) + } + + fun withProtocol() = apply { + every { + callRepository.currentCallProtocol(any()) + }.returns(mlsProtocolInfo) + } + + suspend fun withEstablishedCall() = apply { + coEvery { + callRepository.establishedCallsFlow() + }.returns(flowOf(listOf(call))) + } + + fun withShouldEndSFTOneOnOneCall(result: Boolean) = apply { + every { + mlsCallHelper.shouldEndSFTOneOnOneCall(any(), any(), any(), any(), any()) + }.returns(result) + } + + suspend fun withUserRepositorySuccess() = apply { + coEvery { + userRepository.getKnownUserMinimized(any()) + }.returns( + Either.Right( + OtherUserMinimized( + TestUser.SELF.id, + "name", + null, + UserType.ADMIN + ) + ) + ) + } + } + + companion object { + lateinit var testScope: TestScope + private const val REMOTE_CONVERSATION_ID = "c9mGRDNE7YRVRbk6jokwXNXPgU1n37iS" + private val data = """ + { + "convid": "c9mGRDNE7YRVRbk6jokwXNXPgU1n37iS", + "members": [ + { + "userid": "userid1", + "clientid": "clientid1", + "aestab": "1", + "vrecv": "1", + "muted": "0" + }, + { + "userid": "userid2", + "clientid": "clientid2", + "aestab": "1", + "vrecv": "1", + "muted": "0" + } + ] + } + """ + val participant = Participant( + id = QualifiedID("participantId", "participantDomain"), + clientId = "abcd", + name = "name", + isMuted = true, + isSpeaking = false, + isCameraOn = false, + avatarAssetId = null, + isSharingScreen = false, + hasEstablishedAudio = true + ) + val mlsProtocolInfo = Conversation.ProtocolInfo.MLS( + GroupID("groupid"), + Conversation.ProtocolInfo.MLSCapable.GroupState.ESTABLISHED, + 1UL, + Clock.System.now(), + CipherSuite.MLS_128_DHKEMX25519_AES128GCM_SHA256_Ed25519 + ) + val conversationId = ConversationId("conversationId", "domainId") + + private val call = Call( + conversationId = conversationId, + status = CallStatus.ESTABLISHED, + callerId = "called-id", + isMuted = false, + isCameraOn = false, + isCbrEnabled = false, + conversationName = null, + conversationType = Conversation.Type.ONE_ON_ONE, + callerName = null, + callerTeamName = null, + establishedTime = null + ) + } +} \ No newline at end of file