Skip to content

Commit

Permalink
fix: No SystemMessage on new 1o1 conversation
Browse files Browse the repository at this point in the history
  • Loading branch information
@borichellow
borichellow committed Apr 30, 2024
1 parent 18742df commit 5cd669e
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 116 deletions.
29 changes: 18 additions & 11 deletions ...monMain/kotlin/com/wire/kalium/logic/sync/receiver/conversation/MemberJoinEventHandler.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package com.wire.kalium.logic.sync.receiver.conversation
import com.benasher44.uuid.uuid4
import com.wire.kalium.logger.KaliumLogger
import com.wire.kalium.logic.CoreFailure
import com.wire.kalium.logic.data.conversation.Conversation
import com.wire.kalium.logic.data.conversation.ConversationRepository
import com.wire.kalium.logic.data.event.Event
import com.wire.kalium.logic.data.event.EventLoggingStatus
Expand Down Expand Up @@ -74,17 +75,9 @@ internal class MemberJoinEventHandlerImpl(
userRepository.fetchUsersIfUnknownByIds(event.members.map { it.id }.toSet())
conversationRepository.persistMembers(event.members, event.conversationId)
}.onSuccess {
val message = Message.System(
id = event.id.ifEmpty { uuid4().toString() },
content = MessageContent.MemberChange.Added(members = event.members.map { it.id }),
conversationId = event.conversationId,
date = event.timestampIso,
senderUserId = event.addedBy,
status = Message.Status.Sent,
visibility = Message.Visibility.VISIBLE,
expirationData = null
)
persistMessage(message)
conversationRepository.detailsById(event.conversationId).onSuccess { conversation ->
if (conversation.type == Conversation.Type.GROUP) addSystemMessage(event)
}
kaliumLogger
.logEventProcessing(
EventLoggingStatus.SUCCESS,
Expand All @@ -98,4 +91,18 @@ internal class MemberJoinEventHandlerImpl(
Pair("errorInfo", "$it")
)
}

private suspend fun addSystemMessage(event: Event.Conversation.MemberJoin) {
val message = Message.System(
id = event.id.ifEmpty { uuid4().toString() },
content = MessageContent.MemberChange.Added(members = event.members.map { it.id }),
conversationId = event.conversationId,
date = event.timestampIso,
senderUserId = event.addedBy,
status = Message.Status.Sent,
visibility = Message.Visibility.VISIBLE,
expirationData = null
)
persistMessage(message)
}
}
178 changes: 73 additions & 105 deletions ...est/kotlin/com/wire/kalium/logic/sync/receiver/conversation/MemberJoinEventHandlerTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,24 @@

package com.wire.kalium.logic.sync.receiver.conversation

import com.wire.kalium.logic.CoreFailure
import com.wire.kalium.logic.NetworkFailure
import com.wire.kalium.logic.StorageFailure
import com.wire.kalium.logic.data.conversation.Conversation.Member
import com.wire.kalium.logic.data.conversation.ConversationRepository
import com.wire.kalium.logic.data.message.Message
import com.wire.kalium.logic.data.message.MessageContent
import com.wire.kalium.logic.data.message.PersistMessageUseCase
import com.wire.kalium.logic.data.user.UserRepository
import com.wire.kalium.logic.framework.TestConversation
import com.wire.kalium.logic.framework.TestEvent
import com.wire.kalium.logic.framework.TestUser
import com.wire.kalium.logic.functional.Either
import io.mockative.Mock
import com.wire.kalium.logic.functional.left
import com.wire.kalium.logic.functional.right
import com.wire.kalium.logic.util.arrangement.repository.ConversationRepositoryArrangement
import com.wire.kalium.logic.util.arrangement.repository.ConversationRepositoryArrangementImpl
import com.wire.kalium.logic.util.arrangement.repository.UserRepositoryArrangement
import com.wire.kalium.logic.util.arrangement.repository.UserRepositoryArrangementImpl
import com.wire.kalium.logic.util.arrangement.usecase.PersistMessageUseCaseArrangement
import com.wire.kalium.logic.util.arrangement.usecase.PersistMessageUseCaseArrangementImpl
import io.mockative.any
import io.mockative.classOf
import io.mockative.eq
import io.mockative.given
import io.mockative.matching
import io.mockative.mock
import io.mockative.once
import io.mockative.verify
import kotlinx.coroutines.test.runTest
Expand All @@ -49,12 +48,10 @@ class MemberJoinEventHandlerTest {
val newMembers = listOf(Member(TestUser.OTHER_FEDERATED_USER_ID, Member.Role.Member))
val event = TestEvent.memberJoin(members = newMembers)

val (arrangement, eventHandler) = Arrangement()
.withPersistingMessageReturning(Either.Right(Unit))
.withFetchConversationIfUnknownSucceeding()
.withPersistMembersSucceeding()
.withFetchUsersIfUnknownByIdsReturning(Either.Right(Unit))
.arrange()
val (arrangement, eventHandler) = arrange {
withFetchConversationSucceeding()
withConversationDetailsByIdReturning(TestConversation.CONVERSATION.right())
}

eventHandler.handle(event)

Expand All @@ -74,12 +71,10 @@ class MemberJoinEventHandlerTest {
val newMembers = listOf(Member(TestUser.SELF.id, Member.Role.Member))
val event = TestEvent.memberJoin(members = newMembers)

val (arrangement, eventHandler) = Arrangement()
.withPersistingMessageReturning(Either.Right(Unit))
.withFetchConversationIfUnknownSucceeding()
.withPersistMembersSucceeding()
.withFetchUsersIfUnknownByIdsReturning(Either.Right(Unit))
.arrange()
val (arrangement, eventHandler) = arrange {
withFetchConversationSucceeding()
withConversationDetailsByIdReturning(TestConversation.CONVERSATION.right())
}

eventHandler.handle(event)

Expand All @@ -99,12 +94,10 @@ class MemberJoinEventHandlerTest {
val newMembers = listOf(Member(TestUser.USER_ID, Member.Role.Member))
val event = TestEvent.memberJoin(members = newMembers)

val (arrangement, eventHandler) = Arrangement()
.withPersistingMessageReturning(Either.Right(Unit))
.withFetchConversationIfUnknownSucceeding()
.withPersistMembersSucceeding()
.withFetchUsersIfUnknownByIdsReturning(Either.Right(Unit))
.arrange()
val (arrangement, eventHandler) = arrange {
withFetchConversationSucceeding()
withConversationDetailsByIdReturning(TestConversation.CONVERSATION.right())
}

eventHandler.handle(event)

Expand All @@ -119,12 +112,11 @@ class MemberJoinEventHandlerTest {
val newMembers = listOf(Member(TestUser.USER_ID, Member.Role.Member))
val event = TestEvent.memberJoin(members = newMembers)

val (arrangement, eventHandler) = Arrangement()
.withPersistingMessageReturning(Either.Right(Unit))
.withFetchConversationIfUnknownFailing(NetworkFailure.NoNetworkConnection(null))
.withFetchUsersIfUnknownByIdsReturning(Either.Right(Unit))
.withPersistMembersSucceeding()
.arrange()
val (arrangement, eventHandler) = arrange {
withFetchConversationIfUnknownFailingWith(NetworkFailure.NoNetworkConnection(null))
withFetchConversation(NetworkFailure.NoNetworkConnection(null).left())
withConversationDetailsByIdReturning(TestConversation.CONVERSATION.right())
}

eventHandler.handle(event)

Expand All @@ -139,17 +131,15 @@ class MemberJoinEventHandlerTest {
val newMembers = listOf(Member(TestUser.USER_ID, Member.Role.Admin))
val event = TestEvent.memberJoin(members = newMembers)

val (arrangement, eventHandler) = Arrangement()
.withPersistingMessageReturning(Either.Right(Unit))
.withFetchConversationIfUnknownSucceeding()
.withFetchUsersIfUnknownByIdsReturning(Either.Right(Unit))
.withPersistMembersSucceeding()
.arrange()
val (arrangement, eventHandler) = arrange {
withFetchConversationSucceeding()
withConversationDetailsByIdReturning(TestConversation.GROUP().right())
}

eventHandler.handle(event)

verify(arrangement.persistMessage)
.suspendFunction(arrangement.persistMessage::invoke)
verify(arrangement.persistMessageUseCase)
.suspendFunction(arrangement.persistMessageUseCase::invoke)
.with(
matching {
it is Message.System && it.content is MessageContent.MemberChange
Expand All @@ -158,22 +148,38 @@ class MemberJoinEventHandlerTest {
.wasInvoked(exactly = once)
}

@Test
fun givenMemberJoinEventIn1o1Conversation_whenHandlingIt_thenShouldNotPersistSystemMessage() = runTest {
val newMembers = listOf(Member(TestUser.USER_ID, Member.Role.Admin))
val event = TestEvent.memberJoin(members = newMembers)

val (arrangement, eventHandler) = arrange {
withFetchConversationSucceeding()
withConversationDetailsByIdReturning(TestConversation.CONVERSATION.right())
}

eventHandler.handle(event)

verify(arrangement.persistMessageUseCase)
.suspendFunction(arrangement.persistMessageUseCase::invoke)
.with(any())
.wasNotInvoked()
}

@Test
fun givenMemberJoinEventWithEmptyId_whenHandlingIt_thenShouldPersistSystemMessage() = runTest {
val newMembers = listOf(Member(TestUser.USER_ID, Member.Role.Admin))
val event = TestEvent.memberJoin(members = newMembers).copy(id = "")

val (arrangement, eventHandler) = Arrangement()
.withPersistingMessageReturning(Either.Right(Unit))
.withFetchConversationIfUnknownSucceeding()
.withFetchUsersIfUnknownByIdsReturning(Either.Right(Unit))
.withPersistMembersSucceeding()
.arrange()
val (arrangement, eventHandler) = arrange {
withFetchConversationSucceeding()
withConversationDetailsByIdReturning(TestConversation.GROUP().right())
}

eventHandler.handle(event)

verify(arrangement.persistMessage)
.suspendFunction(arrangement.persistMessage::invoke)
verify(arrangement.persistMessageUseCase)
.suspendFunction(arrangement.persistMessageUseCase::invoke)
.with(
matching {
it is Message.System && it.content is MessageContent.MemberChange && it.id.isNotEmpty()
Expand All @@ -182,67 +188,29 @@ class MemberJoinEventHandlerTest {
.wasInvoked(exactly = once)
}

private class Arrangement {
@Mock
val persistMessage = mock(classOf<PersistMessageUseCase>())

@Mock
val conversationRepository = mock(classOf<ConversationRepository>())

@Mock
private val userRepository = mock(classOf<UserRepository>())

private val memberJoinEventHandler: MemberJoinEventHandler = MemberJoinEventHandlerImpl(
conversationRepository,
userRepository,
persistMessage,
TestUser.SELF.id
)
private class Arrangement(
private val block: Arrangement.() -> Unit
) : ConversationRepositoryArrangement by ConversationRepositoryArrangementImpl(),
UserRepositoryArrangement by UserRepositoryArrangementImpl(),
PersistMessageUseCaseArrangement by PersistMessageUseCaseArrangementImpl() {

fun withPersistingMessageReturning(result: Either<CoreFailure, Unit>) = apply {
given(persistMessage)
.suspendFunction(persistMessage::invoke)
.whenInvokedWith(any())
.thenReturn(result)
fun withFetchConversationSucceeding() = apply {
withFetchConversationIfUnknownSucceeding()
withFetchConversation(Unit.right())
}

fun withFetchConversationIfUnknownSucceeding() = apply {
given(conversationRepository)
.suspendFunction(conversationRepository::fetchConversationIfUnknown)
.whenInvokedWith(any())
.thenReturn(Either.Right(Unit))
given(conversationRepository)
.suspendFunction(conversationRepository::fetchConversation)
.whenInvokedWith(any())
.thenReturn(Either.Right(Unit))
}

fun withFetchConversationIfUnknownFailing(coreFailure: CoreFailure) = apply {
given(conversationRepository)
.suspendFunction(conversationRepository::fetchConversationIfUnknown)
.whenInvokedWith(any())
.thenReturn(Either.Left(coreFailure))
given(conversationRepository)
.suspendFunction(conversationRepository::fetchConversation)
.whenInvokedWith(any())
.thenReturn(Either.Left(coreFailure))
}
fun arrange() = run {
block()

fun withPersistMembersSucceeding() = apply {
given(conversationRepository)
.suspendFunction(conversationRepository::persistMembers)
.whenInvokedWith(any(), any())
.thenReturn(Either.Right(Unit))
}
withPersistingMessage(Unit.right())
withFetchUsersIfUnknownByIdsReturning(Unit.right())
withPersistMembers(Unit.right())

fun withFetchUsersIfUnknownByIdsReturning(result: Either<StorageFailure, Unit>) = apply {
given(userRepository)
.suspendFunction(userRepository::fetchUsersIfUnknownByIds)
.whenInvokedWith(any())
.thenReturn(result)
this to MemberJoinEventHandlerImpl(conversationRepository, userRepository, persistMessageUseCase, TestUser.SELF.id)
}

fun arrange() = this to memberJoinEventHandler
}

private companion object {
fun arrange(block: Arrangement.() -> Unit) = Arrangement(block).arrange()
}
}
8 changes: 8 additions & 0 deletions ...in/com/wire/kalium/logic/util/arrangement/repository/ConversationRepositoryArrangement.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ internal interface ConversationRepositoryArrangement {

fun withSelectGroupStatusMembersNamesAndHandles(result: Either<StorageFailure, EpochChangesData>)
fun withConversationDetailsByIdReturning(result: Either<StorageFailure, Conversation>)
fun withPersistMembers(result: Either<StorageFailure, Unit>)
}

internal open class ConversationRepositoryArrangementImpl : ConversationRepositoryArrangement {
Expand Down Expand Up @@ -278,4 +279,11 @@ internal open class ConversationRepositoryArrangementImpl : ConversationReposito
.whenInvokedWith(any())
.thenReturn(result)
}

override fun withPersistMembers(result: Either<StorageFailure, Unit>) {
given(conversationRepository)
.suspendFunction(conversationRepository::persistMembers)
.whenInvokedWith(any())
.thenReturn(result)
}
}

0 comments on commit 5cd669e

Please sign in to comment.