refactor: optimise legal hold discovery when handling new message and…

… unify fetching clients [WPB-5837] (#2334)

* refactor: optimise legal hold discovery when handling new message and unify fetching clients

* change name

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")

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,

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)

logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/PersistOtherUserClientsUseCase.kt → 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<UserId>)
 }
 
-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<UserId>): Unit =
+        clientRemoteRepository.fetchOtherUserClients(userIdList).fold({
             kaliumLogger.withFeatureId(CLIENTS).e("Failure while fetching other users clients $it")
         }, {
             it.forEach { (userId, clientList) ->

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
@@ -63,7 +63,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,
@@ -163,7 +163,7 @@ internal class LegalHoldHandlerImpl internal constructor(
             userConfigRepository.deleteLegalHoldRequest()
             fetchSelfClientsFromRemote()
         } else {
-            persistOtherUserClients(userId)
+            fetchUsersClientsFromRemote(listOf(userId))
         }
     }
 
@@ -217,13 +217,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)

logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/client/PersistOtherUsersClientsUseCaseTest.kt → 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)
@@ -99,8 +99,8 @@ class PersistOtherUsersClientsUseCaseTest {
 
         val clientMapper = MapperProvider.clientMapper()
 
-        val persistOtherUserClientsUseCase =
-            PersistOtherUserClientsUseCaseImpl(clientRemoteRepository, clientRepository)
+        val fetchUsersClientsFromRemoteUseCase =
+            FetchUsersClientsFromRemoteUseCaseImpl(clientRemoteRepository, clientRepository)
 
         suspend fun withSuccessfulResponse(userIdDTO: UserIdDTO, expectedResponse: List<SimpleClientResponse>): Arrangement {
             given(clientRemoteRepository)
@@ -127,6 +127,6 @@ class PersistOtherUsersClientsUseCaseTest {
             return this
         }
 
-        fun arrange() = this to persistOtherUserClientsUseCase
+        fun arrange() = this to fetchUsersClientsFromRemoteUseCase
     }
 }

logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/handler/legalhold/LegalHoldHandlerTest.kt

@@ -28,7 +28,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
@@ -112,8 +112,8 @@ class LegalHoldHandlerTest {
             .wasNotInvoked()
 
         advanceUntilIdle()
-        verify(arrangement.persistOtherUserClients)
-            .suspendFunction(arrangement.persistOtherUserClients::invoke)
+        verify(arrangement.fetchUsersClientsFromRemote)
+            .suspendFunction(arrangement.fetchUsersClientsFromRemote::invoke)
             .with(any())
             .wasInvoked(once)
     }
@@ -632,7 +632,7 @@ class LegalHoldHandlerTest {
     private class Arrangement {
 
         @Mock
-        val persistOtherUserClients = mock(PersistOtherUserClientsUseCase::class)
+        val fetchUsersClientsFromRemote = mock(FetchUsersClientsFromRemoteUseCase::class)
 
         @Mock
         val fetchSelfClientsFromRemote = mock(FetchSelfClientsFromRemoteUseCase::class)
@@ -668,7 +668,7 @@ class LegalHoldHandlerTest {
         fun arrange() =
             this to LegalHoldHandlerImpl(
                 selfUserId = TestUser.SELF.id,
-                persistOtherUserClients = persistOtherUserClients,
+                fetchUsersClientsFromRemote = fetchUsersClientsFromRemote,
                 fetchSelfClientsFromRemote = fetchSelfClientsFromRemote,
                 observeLegalHoldStateForUser = observeLegalHoldStateForUser,
                 membersHavingLegalHoldClient = membersHavingLegalHoldClient,

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)