diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/GetOrCreateOneToOneConversationUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/GetOrCreateOneToOneConversationUseCase.kt index 8d69c4bf2e6..ee01026bd33 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/GetOrCreateOneToOneConversationUseCase.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/GetOrCreateOneToOneConversationUseCase.kt @@ -33,10 +33,6 @@ import kotlinx.coroutines.flow.first /** * Operation that creates one-to-one Conversation with specific [UserId] (only if it is absent in local DB) * and returns [Conversation] data. - * - * @param otherUserId [UserId] private conversation with which we are interested in. - * @return Result with [Conversation] in case of success, or [CoreFailure] if something went wrong: - * can't get data from local DB, or can't create a conversation. */ interface GetOrCreateOneToOneConversationUseCase { suspend operator fun invoke(otherUserId: UserId): CreateConversationResult @@ -47,6 +43,14 @@ internal class GetOrCreateOneToOneConversationUseCaseImpl( private val userRepository: UserRepository, private val oneOnOneResolver: OneOnOneResolver ) : GetOrCreateOneToOneConversationUseCase { + + /** + * The use case operation operation params and return type. + * + * @param otherUserId [UserId] private conversation with which we are interested in. + * @return Result with [Conversation] in case of success, or [CoreFailure] if something went wrong: + * can't get data from local DB, or can't create a conversation. + */ override suspend operator fun invoke(otherUserId: UserId): CreateConversationResult { // TODO periodically re-resolve one-on-one return conversationRepository.observeOneToOneConversationWithOtherUser(otherUserId) @@ -66,6 +70,18 @@ internal class GetOrCreateOneToOneConversationUseCaseImpl( }) } + /** + * Resolves one-on-one conversation with the user. + * Resolving conversations is the process of: + * + * - Intersecting the supported protocols of the self user and the other user. + * - Selecting the common protocol, based on the team settings with the highest priority. + * - Get or create a conversation with the other user. + * - If the protocol now is MLS, migrate the existing Proteus conversation to MLS. + * - Mark the conversation as active. + * + * If no common protocol is found, and we have existing Proteus conversations, we do best effort to use them as fallback. + */ private suspend fun resolveOneOnOneConversationWithUser(otherUserId: UserId): Either = userRepository.userById(otherUserId).flatMap { otherUser -> // TODO support lazily establishing mls group for team 1-1 diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneMigrator.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneMigrator.kt index 5a9359cb840..5d240c9cf95 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneMigrator.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneMigrator.kt @@ -35,7 +35,19 @@ import com.wire.kalium.logic.functional.map import com.wire.kalium.logic.kaliumLogger interface OneOnOneMigrator { + /** + * Migrates the user's one-on-one Proteus. Without creating a new one since MLS is the default, marking it as active. + */ + suspend fun migrateExistingProteus(user: OtherUser): Either + + /** + * Get one-on-one conversation with the user, if not found, create a new one (Proteus still default) and mark it as active. + */ suspend fun migrateToProteus(user: OtherUser): Either + + /** + * Perform migration of Proteus to MLS keeping history and marking the new conversation as active. + */ suspend fun migrateToMLS(user: OtherUser): Either } @@ -92,19 +104,35 @@ internal class OneOnOneMigratorImpl( } } + override suspend fun migrateExistingProteus(user: OtherUser): Either = + conversationRepository.getOneOnOneConversationsWithOtherUser(user.id, Conversation.Protocol.PROTEUS).flatMap { conversationIds -> + if (conversationIds.isNotEmpty()) { + val conversationId = conversationIds.first() + Either.Right(conversationId) + } else { + Either.Left(StorageFailure.DataNotFound) + } + }.flatMap { conversationId -> + if (user.activeOneOnOneConversationId != conversationId) { + kaliumLogger.d("resolved existing one-on-one to proteus, user = ${user.id.toLogString()}") + userRepository.updateActiveOneOnOneConversation(user.id, conversationId) + } + Either.Right(conversationId) + } + private suspend fun migrateOneOnOneHistory(user: OtherUser, targetConversation: ConversationId): Either { - return conversationRepository.getOneOnOneConversationsWithOtherUser( - otherUserId = user.id, - protocol = Conversation.Protocol.PROTEUS - ).flatMap { proteusOneOnOneConversations -> - // We can theoretically have more than one proteus 1-1 conversation with - // team members since there was no backend safeguards against this - proteusOneOnOneConversations.foldToEitherWhileRight(Unit) { proteusOneOnOneConversation, _ -> - messageRepository.moveMessagesToAnotherConversation( - originalConversation = proteusOneOnOneConversation, - targetConversation = targetConversation - ) - } + return conversationRepository.getOneOnOneConversationsWithOtherUser( + otherUserId = user.id, + protocol = Conversation.Protocol.PROTEUS + ).flatMap { proteusOneOnOneConversations -> + // We can theoretically have more than one proteus 1-1 conversation with + // team members since there was no backend safeguards against this + proteusOneOnOneConversations.foldToEitherWhileRight(Unit) { proteusOneOnOneConversation, _ -> + messageRepository.moveMessagesToAnotherConversation( + originalConversation = proteusOneOnOneConversation, + targetConversation = targetConversation + ) } + } } } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneResolver.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneResolver.kt index c9ba938d312..a136f97107c 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneResolver.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneResolver.kt @@ -31,7 +31,9 @@ import com.wire.kalium.logic.feature.protocol.OneOnOneProtocolSelector import com.wire.kalium.logic.functional.Either import com.wire.kalium.logic.functional.flatMap import com.wire.kalium.logic.functional.flatMapLeft +import com.wire.kalium.logic.functional.fold import com.wire.kalium.logic.functional.foldToEitherWhileRight +import com.wire.kalium.logic.functional.left import com.wire.kalium.logic.functional.map import com.wire.kalium.logic.kaliumLogger import com.wire.kalium.util.KaliumDispatcher @@ -154,11 +156,18 @@ internal class OneOnOneResolverImpl( if (invalidateCurrentKnownProtocols) { userRepository.fetchUsersByIds(setOf(user.id)) } - return oneOnOneProtocolSelector.getProtocolForUser(user.id).flatMap { supportedProtocol -> + return oneOnOneProtocolSelector.getProtocolForUser(user.id).fold({ coreFailure -> + if (coreFailure is CoreFailure.NoCommonProtocolFound.OtherNeedToUpdate) { + kaliumLogger.i("Resolving existing proteus 1:1 as not matching protocol found with: ${user.id.toLogString()}") + oneOnOneMigrator.migrateExistingProteus(user) + } else { + coreFailure.left() + } + }, { supportedProtocol -> when (supportedProtocol) { SupportedProtocol.PROTEUS -> oneOnOneMigrator.migrateToProteus(user) SupportedProtocol.MLS -> oneOnOneMigrator.migrateToMLS(user) } - } + }) } } diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneMigratorTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneMigratorTest.kt index 59c9cef6fab..f8249116d18 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneMigratorTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneMigratorTest.kt @@ -37,9 +37,11 @@ import com.wire.kalium.logic.util.arrangement.repository.UserRepositoryArrangeme import com.wire.kalium.logic.util.shouldFail import com.wire.kalium.logic.util.shouldSucceed import io.mockative.any +import io.mockative.coVerify import io.mockative.eq import io.mockative.once import io.mockative.verify +import io.mockative.verify import kotlinx.coroutines.test.runTest import kotlin.test.Test import kotlin.test.assertEquals @@ -133,6 +135,11 @@ class OneOnOneMigratorTest { .suspendFunction(arrangement.messageRepository::moveMessagesToAnotherConversation) .with(any(), any()) .wasNotInvoked() + + verify(arrangement.systemMessageInserter) + .suspendFunction(arrangement.systemMessageInserter::insertProtocolChangedSystemMessage) + .with(any(), any(), any()) + .wasNotInvoked() } @Test @@ -142,7 +149,7 @@ class OneOnOneMigratorTest { ) val failure = CoreFailure.MissingClientRegistration - val (_, oneOnOneMigrator) = arrange { + val (arrangement, oneOnOneMigrator) = arrange { withResolveConversationReturning(Either.Left(failure)) } @@ -173,6 +180,11 @@ class OneOnOneMigratorTest { .suspendFunction(arrangement.userRepository::updateActiveOneOnOneConversation) .with(any(), any()) .wasNotInvoked() + + verify(arrangement.systemMessageInserter) + .suspendFunction(arrangement.systemMessageInserter::insertProtocolChangedSystemMessage) + .with(any(), any(), any()) + .wasNotInvoked() } @Test @@ -215,6 +227,11 @@ class OneOnOneMigratorTest { .suspendFunction(arrangement.messageRepository::moveMessagesToAnotherConversation) .with(eq(originalConversationId), eq(resolvedConversationId)) .wasInvoked(exactly = once) + + verify(arrangement.systemMessageInserter) + .suspendFunction(arrangement.systemMessageInserter::insertProtocolChangedSystemMessage) + .with(any(), any(), any()) + .wasInvoked(exactly = once) } @Test @@ -238,15 +255,44 @@ class OneOnOneMigratorTest { .suspendFunction(arrangement.userRepository::updateActiveOneOnOneConversation) .with(eq(user.id), eq(resolvedConversationId)) .wasInvoked(exactly = once) + + verify(arrangement.systemMessageInserter) + .suspendFunction(arrangement.systemMessageInserter::insertProtocolChangedSystemMessage) + .with(any(), any(), any()) + .wasInvoked(exactly = once) + } + + @Test + fun givenExistingTeamOneOnOne_whenMigratingToProteus_thenShouldNOTCreateGroupConversation() = runTest { + val user = TestUser.OTHER.copy( + activeOneOnOneConversationId = null + ) + + val (arrangement, oneOneMigrator) = arrange { + withGetOneOnOneConversationsWithOtherUserReturning(Either.Right(listOf(TestConversation.ONE_ON_ONE().id))) + withUpdateOneOnOneConversationReturning(Either.Right(Unit)) + } + + oneOneMigrator.migrateExistingProteus(user) + .shouldSucceed() + + verify(arrangement.conversationGroupRepository) + .suspendFunction(arrangement.conversationGroupRepository::createGroupConversation) + .with(eq(null), eq(listOf(TestUser.OTHER.id)), eq(ConversationOptions())) + .wasNotInvoked() + + verify(arrangement.userRepository) + .suspendFunction(arrangement.userRepository::updateActiveOneOnOneConversation) + .with(eq(TestUser.OTHER.id), eq(TestConversation.ONE_ON_ONE().id)) + .wasInvoked(exactly = once) } - private class Arrangement(private val block: Arrangement.() -> Unit) : + private class Arrangement(private val block: suspend Arrangement.() -> Unit) : MLSOneOnOneConversationResolverArrangement by MLSOneOnOneConversationResolverArrangementImpl(), MessageRepositoryArrangement by MessageRepositoryArrangementImpl(), ConversationRepositoryArrangement by ConversationRepositoryArrangementImpl(), ConversationGroupRepositoryArrangement by ConversationGroupRepositoryArrangementImpl(), - UserRepositoryArrangement by UserRepositoryArrangementImpl() - { + UserRepositoryArrangement by UserRepositoryArrangementImpl() { fun arrange() = run { block() this@Arrangement to OneOnOneMigratorImpl( @@ -254,7 +300,8 @@ class OneOnOneMigratorTest { conversationGroupRepository = conversationGroupRepository, conversationRepository = conversationRepository, messageRepository = messageRepository, - userRepository = userRepository + userRepository = userRepository, + systemMessageInserter = systemMessageInserter ) } } diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneResolverTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneResolverTest.kt index 75487a96df9..8625cd6ce7d 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneResolverTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneResolverTest.kt @@ -22,6 +22,8 @@ import com.wire.kalium.logic.data.user.SupportedProtocol import com.wire.kalium.logic.framework.TestConversation import com.wire.kalium.logic.framework.TestUser import com.wire.kalium.logic.functional.Either +import com.wire.kalium.logic.functional.left +import com.wire.kalium.logic.functional.right import com.wire.kalium.logic.util.arrangement.IncrementalSyncRepositoryArrangement import com.wire.kalium.logic.util.arrangement.IncrementalSyncRepositoryArrangementImpl import com.wire.kalium.logic.util.arrangement.mls.OneOnOneMigratorArrangement @@ -33,6 +35,7 @@ import com.wire.kalium.logic.util.arrangement.repository.UserRepositoryArrangeme import com.wire.kalium.logic.util.shouldFail import com.wire.kalium.logic.util.shouldSucceed import io.mockative.any +import io.mockative.coVerify import io.mockative.eq import io.mockative.given import io.mockative.matchers.OneOfMatcher @@ -255,6 +258,24 @@ class OneOnOneResolverTest { .wasInvoked(exactly = once) } + @Test + fun givenProtocolResolvesToOtherNeedToUpdate_whenResolveOneOnOneConversationWithUser_thenMigrateExistingToProteus() = runTest { + // given + val (arrangement, resolver) = arrange { + withGetProtocolForUser(CoreFailure.NoCommonProtocolFound.OtherNeedToUpdate.left()) + withMigrateExistingToProteusReturns(TestConversation.ID.right()) + } + + // when + resolver.resolveOneOnOneConversationWithUser(OTHER_USER, false).shouldSucceed() + + // then + verify(arrangement.oneOnOneMigrator) + .suspendFunction(arrangement.oneOnOneMigrator::migrateExistingProteus) + .with(eq(OTHER_USER)) + .wasInvoked(exactly = once) + } + private class Arrangement(private val block: Arrangement.() -> Unit) : UserRepositoryArrangement by UserRepositoryArrangementImpl(), OneOnOneProtocolSelectorArrangement by OneOnOneProtocolSelectorArrangementImpl(), diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/util/arrangement/mls/OneOnOneMigratorArrangement.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/util/arrangement/mls/OneOnOneMigratorArrangement.kt index fbe1d41e3c5..53818e71c14 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/util/arrangement/mls/OneOnOneMigratorArrangement.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/util/arrangement/mls/OneOnOneMigratorArrangement.kt @@ -33,6 +33,8 @@ interface OneOnOneMigratorArrangement { fun withMigrateToMLSReturns(result: Either) fun withMigrateToProteusReturns(result: Either) + + fun withMigrateExistingToProteusReturns(result: Either) } class OneOnOneMigratorArrangementImpl : OneOnOneMigratorArrangement { @@ -53,4 +55,12 @@ class OneOnOneMigratorArrangementImpl : OneOnOneMigratorArrangement { .whenInvokedWith(any()) .thenReturn(result) } + + override fun withMigrateExistingToProteusReturns(result: Either) { + given(oneOnOneMigrator) + .suspendFunction(oneOnOneMigrator::migrateExistingProteus) + .whenInvokedWith(any()) + .thenReturn(result) + } } +//,