From ae5c9d3905b55ea6b1542cd3de8e11ed63a95666 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Wed, 27 Dec 2023 17:38:23 +0100 Subject: [PATCH 1/2] refactor: optimise legal hold discovery when handling new message and unify fetching clients --- .../kalium/cli/commands/DeleteClientCommand.kt | 2 +- .../kalium/logic/feature/UserSessionScope.kt | 10 +++++----- .../kalium/logic/feature/client/ClientScope.kt | 15 +++++---------- ....kt => FetchUsersClientsFromRemoteUseCase.kt} | 12 ++++++------ .../handler/legalhold/LegalHoldHandler.kt | 12 ++++++------ ...=> FetchUsersClientsFromRemoteUseCaseTest.kt} | 16 ++++++++-------- .../handler/legalhold/LegalHoldHandlerTest.kt | 10 +++++----- .../integrationTest/kotlin/PocIntegrationTest.kt | 2 +- 8 files changed, 37 insertions(+), 42 deletions(-) rename logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/{PersistOtherUserClientsUseCase.kt => FetchUsersClientsFromRemoteUseCase.kt} (84%) rename logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/client/{PersistOtherUsersClientsUseCaseTest.kt => FetchUsersClientsFromRemoteUseCaseTest.kt} (91%) diff --git a/cli/src/commonMain/kotlin/com/wire/kalium/cli/commands/DeleteClientCommand.kt b/cli/src/commonMain/kotlin/com/wire/kalium/cli/commands/DeleteClientCommand.kt index 86cda47fbcb..0db663a3a44 100644 --- a/cli/src/commonMain/kotlin/com/wire/kalium/cli/commands/DeleteClientCommand.kt +++ b/cli/src/commonMain/kotlin/com/wire/kalium/cli/commands/DeleteClientCommand.kt @@ -35,7 +35,7 @@ class DeleteClientCommand : CliktCommand(name = "delete-client") { private val password: String by option(help = "Account password").prompt("password", promptSuffix = ": ", hideInput = true) override fun run() = runBlocking { - val selfClientsResult = userSession.client.selfClients() + val selfClientsResult = userSession.client.fetchSelfClients() if (selfClientsResult !is SelfClientsResult.Success) { throw PrintMessage("failed to retrieve self clients") diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt index e479264db7a..5102ef60445 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt @@ -166,12 +166,12 @@ import com.wire.kalium.logic.feature.call.usecase.UpdateConversationClientsForCu import com.wire.kalium.logic.feature.client.ClientScope import com.wire.kalium.logic.feature.client.FetchSelfClientsFromRemoteUseCase import com.wire.kalium.logic.feature.client.FetchSelfClientsFromRemoteUseCaseImpl +import com.wire.kalium.logic.feature.client.FetchUsersClientsFromRemoteUseCase +import com.wire.kalium.logic.feature.client.FetchUsersClientsFromRemoteUseCaseImpl import com.wire.kalium.logic.feature.client.IsAllowedToRegisterMLSClientUseCase import com.wire.kalium.logic.feature.client.IsAllowedToRegisterMLSClientUseCaseImpl import com.wire.kalium.logic.feature.client.MLSClientManager import com.wire.kalium.logic.feature.client.MLSClientManagerImpl -import com.wire.kalium.logic.feature.client.PersistOtherUserClientsUseCase -import com.wire.kalium.logic.feature.client.PersistOtherUserClientsUseCaseImpl import com.wire.kalium.logic.feature.client.RegisterMLSClientUseCaseImpl import com.wire.kalium.logic.feature.connection.ConnectionScope import com.wire.kalium.logic.feature.connection.SyncConnectionsUseCase @@ -1378,8 +1378,8 @@ class UserSessionScope internal constructor( clientRepository = clientRepository, provideClientId = clientIdProvider ) - private val persistOtherUserClients: PersistOtherUserClientsUseCase - get() = PersistOtherUserClientsUseCaseImpl( + private val fetchUsersClientsFromRemote: FetchUsersClientsFromRemoteUseCase + get() = FetchUsersClientsFromRemoteUseCaseImpl( clientRemoteRepository = clientRemoteRepository, clientRepository = clientRepository ) @@ -1396,7 +1396,7 @@ class UserSessionScope internal constructor( private val legalHoldHandler = LegalHoldHandlerImpl( selfUserId = userId, - persistOtherUserClients = persistOtherUserClients, + fetchUsersClientsFromRemote = fetchUsersClientsFromRemote, fetchSelfClientsFromRemote = fetchSelfClientsFromRemote, observeLegalHoldStateForUser = observeLegalHoldStateForUser, membersHavingLegalHoldClient = membersHavingLegalHoldClient, diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/ClientScope.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/ClientScope.kt index 99a7955a1b6..abe6bf7feb8 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/ClientScope.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/ClientScope.kt @@ -85,7 +85,11 @@ class ClientScope @OptIn(DelicateKaliumApi::class) internal constructor( secondFactorVerificationRepository ) - val selfClients: FetchSelfClientsFromRemoteUseCase get() = FetchSelfClientsFromRemoteUseCaseImpl(clientRepository, clientIdProvider) + val fetchSelfClients: FetchSelfClientsFromRemoteUseCase + get() = FetchSelfClientsFromRemoteUseCaseImpl(clientRepository, clientIdProvider) + val fetchUsersClients: FetchUsersClientsFromRemoteUseCase + get() = FetchUsersClientsFromRemoteUseCaseImpl(clientRemoteRepository, clientRepository) + val getOtherUserClients: ObserveClientsByUserIdUseCase get() = ObserveClientsByUserIdUseCase(clientRepository) val observeClientDetailsUseCase: ObserveClientDetailsUseCase get() = ObserveClientDetailsUseCaseImpl(clientRepository, clientIdProvider) val deleteClient: DeleteClientUseCase get() = DeleteClientUseCaseImpl( @@ -106,15 +110,6 @@ class ClientScope @OptIn(DelicateKaliumApi::class) internal constructor( keyPackageLimitsProvider, clientIdProvider ) - val persistOtherUserClients: PersistOtherUserClientsUseCase - get() = PersistOtherUserClientsUseCaseImpl( - clientRemoteRepository, - clientRepository - ) - val getOtherUserClients: ObserveClientsByUserIdUseCase - get() = ObserveClientsByUserIdUseCase( - clientRepository - ) val observeCurrentClientId: ObserveCurrentClientIdUseCase get() = ObserveCurrentClientIdUseCaseImpl(clientRepository) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/PersistOtherUserClientsUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/FetchUsersClientsFromRemoteUseCase.kt similarity index 84% rename from logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/PersistOtherUserClientsUseCase.kt rename to logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/FetchUsersClientsFromRemoteUseCase.kt index cd6849e7881..5312c3bbdf7 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/PersistOtherUserClientsUseCase.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/FetchUsersClientsFromRemoteUseCase.kt @@ -30,17 +30,17 @@ import com.wire.kalium.logic.kaliumLogger /** * Use case to get the other users clients (devices) from remote and save it in our local db so it can be fetched later */ -interface PersistOtherUserClientsUseCase { - suspend operator fun invoke(userId: UserId) +interface FetchUsersClientsFromRemoteUseCase { + suspend operator fun invoke(userIdList: List) } -internal class PersistOtherUserClientsUseCaseImpl( +internal class FetchUsersClientsFromRemoteUseCaseImpl( private val clientRemoteRepository: ClientRemoteRepository, private val clientRepository: ClientRepository, private val clientMapper: ClientMapper = MapperProvider.clientMapper() -) : PersistOtherUserClientsUseCase { - override suspend operator fun invoke(userId: UserId): Unit = - clientRemoteRepository.fetchOtherUserClients(listOf(userId)).fold({ +) : FetchUsersClientsFromRemoteUseCase { + override suspend operator fun invoke(userIdList: List): Unit = + clientRemoteRepository.fetchOtherUserClients(userIdList).fold({ kaliumLogger.withFeatureId(CLIENTS).e("Failure while fetching other users clients $it") }, { it.forEach { (userId, clientList) -> diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/handler/legalhold/LegalHoldHandler.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/handler/legalhold/LegalHoldHandler.kt index d2fe61709c8..ea0218d5021 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/handler/legalhold/LegalHoldHandler.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/handler/legalhold/LegalHoldHandler.kt @@ -26,7 +26,7 @@ import com.wire.kalium.logic.data.id.ConversationId import com.wire.kalium.logic.data.sync.SyncState import com.wire.kalium.logic.data.user.UserId import com.wire.kalium.logic.feature.client.FetchSelfClientsFromRemoteUseCase -import com.wire.kalium.logic.feature.client.PersistOtherUserClientsUseCase +import com.wire.kalium.logic.feature.client.FetchUsersClientsFromRemoteUseCase import com.wire.kalium.logic.feature.legalhold.LegalHoldState import com.wire.kalium.logic.feature.legalhold.MembersHavingLegalHoldClientUseCase import com.wire.kalium.logic.feature.legalhold.ObserveLegalHoldStateForUserUseCase @@ -57,7 +57,7 @@ internal interface LegalHoldHandler { @Suppress("LongParameterList") internal class LegalHoldHandlerImpl internal constructor( private val selfUserId: UserId, - private val persistOtherUserClients: PersistOtherUserClientsUseCase, + private val fetchUsersClientsFromRemote: FetchUsersClientsFromRemoteUseCase, private val fetchSelfClientsFromRemote: FetchSelfClientsFromRemoteUseCase, private val observeLegalHoldStateForUser: ObserveLegalHoldStateForUserUseCase, private val membersHavingLegalHoldClient: MembersHavingLegalHoldClientUseCase, @@ -132,7 +132,7 @@ internal class LegalHoldHandlerImpl internal constructor( userConfigRepository.deleteLegalHoldRequest() fetchSelfClientsFromRemote() } else { - persistOtherUserClients(userId) + fetchUsersClientsFromRemote(listOf(userId)) } } @@ -186,13 +186,13 @@ internal class LegalHoldHandlerImpl internal constructor( } } .map { + fetchUsersClientsFromRemote(it.keys.toList()) it.forEach { (userId, userHasBeenUnderLegalHold) -> - // TODO: to be optimized - send empty message and handle legal hold discovery after sending a message - processEvent(selfUserId, userId) val userIsNowUnderLegalHold = isUserUnderLegalHold(userId) if (userHasBeenUnderLegalHold != userIsNowUnderLegalHold) { - if (selfUserId == userId) { // notify only for self user + if (selfUserId == userId) { // notify and delete request only for self user userConfigRepository.setLegalHoldChangeNotified(false) + userConfigRepository.deleteLegalHoldRequest() } if (userIsNowUnderLegalHold) legalHoldSystemMessagesHandler.handleEnabledForUser(userId, systemMessageTimestampIso) else legalHoldSystemMessagesHandler.handleDisabledForUser(userId, systemMessageTimestampIso) diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/client/PersistOtherUsersClientsUseCaseTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/client/FetchUsersClientsFromRemoteUseCaseTest.kt similarity index 91% rename from logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/client/PersistOtherUsersClientsUseCaseTest.kt rename to logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/client/FetchUsersClientsFromRemoteUseCaseTest.kt index 78e227ef5ea..ea88b75bab9 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/client/PersistOtherUsersClientsUseCaseTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/client/FetchUsersClientsFromRemoteUseCaseTest.kt @@ -27,7 +27,6 @@ import com.wire.kalium.logic.functional.Either import com.wire.kalium.logic.test_util.TestNetworkException import com.wire.kalium.network.api.base.authenticated.client.DeviceTypeDTO import com.wire.kalium.network.api.base.authenticated.client.SimpleClientResponse -import com.wire.kalium.network.api.base.model.UserId as UserIdDTO import com.wire.kalium.network.exceptions.KaliumException import io.mockative.Mock import io.mockative.any @@ -39,9 +38,10 @@ import io.mockative.verify import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import kotlin.test.Test +import com.wire.kalium.network.api.base.model.UserId as UserIdDTO @ExperimentalCoroutinesApi -class PersistOtherUsersClientsUseCaseTest { +class FetchUsersClientsFromRemoteUseCaseTest { @Test fun givenASuccessfulRepositoryResponse_whenInvokingTheUseCase_thenSuccessResultIsReturned() = runTest { @@ -52,12 +52,12 @@ class PersistOtherUsersClientsUseCaseTest { val otherUserClients = listOf( SimpleClientResponse("111", DeviceTypeDTO.Phone), SimpleClientResponse("2222", DeviceTypeDTO.Desktop) ) - val (arrangement, getOtherUsersClientsUseCase) = Arrangement() + val (arrangement, useCase) = Arrangement() .withSuccessfulResponse(userIdDTO, otherUserClients) .arrange() // When - getOtherUsersClientsUseCase(userId) + useCase(listOf(userId)) verify(arrangement.clientRemoteRepository) .suspendFunction(arrangement.clientRemoteRepository::fetchOtherUserClients).with(any()) @@ -71,16 +71,16 @@ class PersistOtherUsersClientsUseCaseTest { } @Test - fun givenRepositoryCallFailWithInvaliUserId_thenNoUserFoundReturned() = runTest { + fun givenRepositoryCallFailWithInvalidUserId_thenNoUserFoundReturned() = runTest { // Given val userId = UserId("123", "wire.com") val noUserFoundException = TestNetworkException.noTeam - val (arrangement, getOtherUsersClientsUseCase) = Arrangement() + val (arrangement, useCase) = Arrangement() .withGetOtherUserClientsErrorResponse(noUserFoundException) .arrange() // When - getOtherUsersClientsUseCase.invoke(userId) + useCase.invoke(listOf(userId)) // Then verify(arrangement.clientRemoteRepository) @@ -100,7 +100,7 @@ class PersistOtherUsersClientsUseCaseTest { val clientMapper = MapperProvider.clientMapper() val persistOtherUserClientsUseCase = - PersistOtherUserClientsUseCaseImpl(clientRemoteRepository, clientRepository) + FetchUsersClientsFromRemoteUseCaseImpl(clientRemoteRepository, clientRepository) suspend fun withSuccessfulResponse(userIdDTO: UserIdDTO, expectedResponse: List): Arrangement { given(clientRemoteRepository) diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/handler/legalhold/LegalHoldHandlerTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/handler/legalhold/LegalHoldHandlerTest.kt index 8acbe380534..98d480ce5d2 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/handler/legalhold/LegalHoldHandlerTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/handler/legalhold/LegalHoldHandlerTest.kt @@ -27,7 +27,7 @@ import com.wire.kalium.logic.data.message.ProtoContent import com.wire.kalium.logic.data.sync.SyncState import com.wire.kalium.logic.data.user.UserId import com.wire.kalium.logic.feature.client.FetchSelfClientsFromRemoteUseCase -import com.wire.kalium.logic.feature.client.PersistOtherUserClientsUseCase +import com.wire.kalium.logic.feature.client.FetchUsersClientsFromRemoteUseCase import com.wire.kalium.logic.feature.client.SelfClientsResult import com.wire.kalium.logic.feature.legalhold.LegalHoldState import com.wire.kalium.logic.feature.legalhold.MembersHavingLegalHoldClientUseCase @@ -106,8 +106,8 @@ class LegalHoldHandlerTest { .wasNotInvoked() advanceUntilIdle() - verify(arrangement.persistOtherUserClients) - .suspendFunction(arrangement.persistOtherUserClients::invoke) + verify(arrangement.fetchUsersClientsFromRemote) + .suspendFunction(arrangement.fetchUsersClientsFromRemote::invoke) .with(any()) .wasInvoked(once) } @@ -487,7 +487,7 @@ class LegalHoldHandlerTest { private class Arrangement { @Mock - val persistOtherUserClients = mock(PersistOtherUserClientsUseCase::class) + val fetchUsersClientsFromRemote = mock(FetchUsersClientsFromRemoteUseCase::class) @Mock val fetchSelfClientsFromRemote = mock(FetchSelfClientsFromRemoteUseCase::class) @@ -523,7 +523,7 @@ class LegalHoldHandlerTest { fun arrange() = this to LegalHoldHandlerImpl( selfUserId = TestUser.SELF.id, - persistOtherUserClients = persistOtherUserClients, + fetchUsersClientsFromRemote = fetchUsersClientsFromRemote, fetchSelfClientsFromRemote = fetchSelfClientsFromRemote, observeLegalHoldStateForUser = observeLegalHoldStateForUser, membersHavingLegalHoldClient = membersHavingLegalHoldClient, diff --git a/tango-tests/src/integrationTest/kotlin/PocIntegrationTest.kt b/tango-tests/src/integrationTest/kotlin/PocIntegrationTest.kt index 3d831e2fd4c..6c7020de4e9 100644 --- a/tango-tests/src/integrationTest/kotlin/PocIntegrationTest.kt +++ b/tango-tests/src/integrationTest/kotlin/PocIntegrationTest.kt @@ -98,7 +98,7 @@ class PocIntegrationTest { coreLogic = coreLogic ) - val x = userSessionScope.client.selfClients() + val x = userSessionScope.client.fetchSelfClients() println(x.toString()) userSessionScope.logout.invoke(LogoutReason.SELF_SOFT_LOGOUT) From 942e17a5ffa140cc65c5a5dec7c155bae4a08182 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Thu, 28 Dec 2023 13:38:27 +0100 Subject: [PATCH 2/2] change name --- .../feature/client/FetchUsersClientsFromRemoteUseCaseTest.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/client/FetchUsersClientsFromRemoteUseCaseTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/client/FetchUsersClientsFromRemoteUseCaseTest.kt index ea88b75bab9..ceb07598c8d 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/client/FetchUsersClientsFromRemoteUseCaseTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/client/FetchUsersClientsFromRemoteUseCaseTest.kt @@ -99,7 +99,7 @@ class FetchUsersClientsFromRemoteUseCaseTest { val clientMapper = MapperProvider.clientMapper() - val persistOtherUserClientsUseCase = + val fetchUsersClientsFromRemoteUseCase = FetchUsersClientsFromRemoteUseCaseImpl(clientRemoteRepository, clientRepository) suspend fun withSuccessfulResponse(userIdDTO: UserIdDTO, expectedResponse: List): Arrangement { @@ -127,6 +127,6 @@ class FetchUsersClientsFromRemoteUseCaseTest { return this } - fun arrange() = this to persistOtherUserClientsUseCase + fun arrange() = this to fetchUsersClientsFromRemoteUseCase } }