Skip to content

Commit

Permalink
fix: delete asset file when removing self-deleting msg [WPB-1807]
Browse files Browse the repository at this point in the history
  • Loading branch information
@saleniuk
saleniuk committed Sep 24, 2024
1 parent ed2bd50 commit 2df0159
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 81 deletions.
54 changes: 27 additions & 27 deletions ...ium/logic/feature/message/ephemeral/DeleteEphemeralMessageForSelfUserAsReceiverUseCase.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,30 +62,34 @@ internal class DeleteEphemeralMessageForSelfUserAsReceiverUseCaseImpl(
private val selfUserId: UserId,
private val selfConversationIdProvider: SelfConversationIdProvider
) : DeleteEphemeralMessageForSelfUserAsReceiverUseCase {

override suspend fun invoke(conversationId: ConversationId, messageId: String): Either<CoreFailure, Unit> =
messageRepository.markMessageAsDeleted(messageId, conversationId).flatMap {
currentClientIdProvider().flatMap { currentClientId ->
messageRepository.getMessageById(conversationId, messageId)
.flatMap { message ->
sendDeleteMessageToSelf(
message.id,
conversationId,
currentClientId
).flatMap {
sendDeleteMessageToOriginalSender(
message.id,
message.conversationId,
message.senderUserId,
currentClientId
)
}.onSuccess {
deleteMessageAssetIfExists(message)
}.flatMap {
messageRepository.deleteMessage(messageId, conversationId)
}
messageRepository.getMessageById(conversationId, messageId)
.onSuccess { message ->
deleteMessageAssetLocallyIfExists(message)
}
.flatMap { message ->
messageRepository.markMessageAsDeleted(messageId, conversationId)
.flatMap {
currentClientIdProvider()
.flatMap { currentClientId ->
sendDeleteMessageToSelf(
message.id,
conversationId,
currentClientId
).flatMap {
sendDeleteMessageToOriginalSender(
message.id,
message.conversationId,
message.senderUserId,
currentClientId
)
}.flatMap {
messageRepository.deleteMessage(messageId, conversationId)
}
}
}
}
}

private suspend fun sendDeleteMessageToSelf(
messageToDelete: String,
Expand Down Expand Up @@ -131,13 +135,9 @@ internal class DeleteEphemeralMessageForSelfUserAsReceiverUseCaseImpl(
)
}

private suspend fun deleteMessageAssetIfExists(message: Message) {
private suspend fun deleteMessageAssetLocallyIfExists(message: Message) {
(message.content as? MessageContent.Asset)?.value?.remoteData?.let { assetToRemove ->
assetRepository.deleteAsset(
assetToRemove.assetId,
assetToRemove.assetDomain,
assetToRemove.assetToken
).onFailure {
assetRepository.deleteAssetLocally(assetToRemove.assetId).onFailure {
kaliumLogger.withFeatureId(ASSETS).w("delete message asset failure: $it")
}
}
Expand Down
41 changes: 18 additions & 23 deletions ...kotlin/com/wire/kalium/logic/feature/message/ephemeral/EphemeralMessageDeletionHandler.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,43 +62,38 @@ internal class EphemeralMessageDeletionHandlerImpl(

private val ongoingSelfDeletionMessagesMutex = Mutex()
private val ongoingSelfDeletionMessages = mutableMapOf<Pair<ConversationId, String>, Unit>()

override fun startSelfDeletion(conversationId: ConversationId, messageId: String) {
launch {
messageRepository.getMessageById(conversationId, messageId).map { message ->
if (message.expirationData != null && message.status != Message.Status.Pending) {
enqueueSelfDeletion(
message = message,
expirationData = message.expirationData!!
)
} else {
kaliumLogger.i(
"Self deletion requested for message without expiration data: ${message.content.getType()}"
)
val expirationData = message.expirationData
when {
expirationData == null ->
kaliumLogger.i("Self deletion requested for message without expiration data: ${message.content.getType()}")

message.status == Message.Status.Pending ->
logger.log(LoggingSelfDeletionEvent.InvalidMessageStatus(message, expirationData))

else -> enqueueSelfDeletion(message, expirationData)
}
}
}
}

override fun enqueueSelfDeletion(message: Message, expirationData: Message.ExpirationData) {
logger.log(
LoggingSelfDeletionEvent.InvalidMessageStatus(
message,
expirationData
)
)

launch {
ongoingSelfDeletionMessagesMutex.withLock {
val isSelfDeletionOutgoing = ongoingSelfDeletionMessages[message.conversationId to message.id] != null

logger.log(
LoggingSelfDeletionEvent.SelfSelfDeletionAlreadyRequested(
message,
expirationData
if (isSelfDeletionOutgoing) {
logger.log(
LoggingSelfDeletionEvent.SelfSelfDeletionAlreadyRequested(
message,
expirationData
)
)
)

if (isSelfDeletionOutgoing) return@launch
return@launch
}

addToOutgoingDeletion(message)
}
Expand Down
1 change: 1 addition & 0 deletions ...monMain/kotlin/com/wire/kalium/logic/feature/message/ephemeral/SelfDeletionEventLogger.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ internal sealed class LoggingSelfDeletionEvent(
fun toJson(): String {
return EPHEMERAL_LOG_TAG + mapOf(
"message" to (message as Message.Sendable).toLogMap(),
"expiration-data" to expirationData.toLogMap(),
)
.toMutableMap()
.plus(toLogMap())
Expand Down
91 changes: 66 additions & 25 deletions ...um/logic/data/message/ephemeral/DeleteEphemeralMessageForSelfUserAsReceiverUseCaseTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@ package com.wire.kalium.logic.data.message.ephemeral

import com.wire.kalium.logic.data.conversation.ClientId
import com.wire.kalium.logic.data.id.ConversationId
import com.wire.kalium.logic.data.message.AssetContent
import com.wire.kalium.logic.data.message.Message
import com.wire.kalium.logic.data.message.MessageContent
import com.wire.kalium.logic.data.user.UserId
import com.wire.kalium.logic.data.message.MessageEncryptionAlgorithm
import com.wire.kalium.logic.data.message.MessageTarget
import com.wire.kalium.logic.data.user.UserId
import com.wire.kalium.logic.feature.message.ephemeral.DeleteEphemeralMessageForSelfUserAsReceiverUseCase
import com.wire.kalium.logic.feature.message.ephemeral.DeleteEphemeralMessageForSelfUserAsReceiverUseCaseImpl
import com.wire.kalium.logic.functional.Either
Expand All @@ -37,10 +39,10 @@ import com.wire.kalium.logic.util.arrangement.repository.AssetRepositoryArrangem
import com.wire.kalium.logic.util.arrangement.repository.MessageRepositoryArrangement
import com.wire.kalium.logic.util.arrangement.repository.MessageRepositoryArrangementImpl
import com.wire.kalium.logic.util.shouldSucceed
import com.wire.kalium.util.DateTimeUtil.toIsoDateTimeString
import io.mockative.any
import io.mockative.matches
import io.mockative.coVerify
import io.mockative.matchers.EqualsMatcher
import io.mockative.matches
import io.mockative.once
import kotlinx.coroutines.test.runTest
import kotlinx.datetime.Instant
Expand All @@ -50,27 +52,15 @@ class DeleteEphemeralMessageForSelfUserAsReceiverUseCaseTest {

@Test
fun givenMessage_whenDeleting_then2DeleteMessagesAreSentForSelfAndOriginalSender() = runTest {
val messageId = "messageId"
val conversationId = ConversationId("conversationId", "conversationDomain.com")
val currentClientId = CURRENT_CLIENT_ID

val senderUserID = UserId("senderUserId", "senderUserDomain.com")
val message = Message.Regular(
id = messageId,
content = MessageContent.Text("text"),
conversationId = conversationId,
date = Instant.DISTANT_FUTURE,
senderUserId = senderUserID,
senderClientId = currentClientId,
status = Message.Status.Pending,
editStatus = Message.EditStatus.NotEdited,
isSelfMessage = true
)
val message = MESSAGE_REGULAR
val messageId = message.id
val conversationId = message.conversationId
val senderUserId = message.senderUserId
val (arrangement, useCase) = Arrangement()
.arrange {
withMarkAsDeleted(Either.Right(Unit))
withCurrentClientIdSuccess(CURRENT_CLIENT_ID)
withSelfConversationIds(SELF_CONVERSION_ID)
withSelfConversationIds(SELF_CONVERSATION_ID)
withGetMessageById(Either.Right(message))
withSendMessageSucceed()
withDeleteMessage(Either.Right(Unit))
Expand All @@ -85,7 +75,7 @@ class DeleteEphemeralMessageForSelfUserAsReceiverUseCaseTest {
coVerify {
arrangement.messageSender.sendMessage(
matches {
it.conversationId == SELF_CONVERSION_ID.first() &&
it.conversationId == SELF_CONVERSATION_ID.first() &&
it.content == MessageContent.DeleteForMe(messageId, conversationId)
},
matches {
Expand All @@ -101,7 +91,7 @@ class DeleteEphemeralMessageForSelfUserAsReceiverUseCaseTest {
it.content == MessageContent.DeleteMessage(messageId)
},
matches {
it == MessageTarget.Users(listOf(senderUserID))
it == MessageTarget.Users(listOf(senderUserId))
}
)
}.wasInvoked(exactly = once)
Expand All @@ -111,10 +101,61 @@ class DeleteEphemeralMessageForSelfUserAsReceiverUseCaseTest {
}.wasInvoked(exactly = once)
}

@Test
fun givenAssetMessage_whenDeleting_thenDeleteAssetLocally() = runTest {
val assetContent = ASSET_IMAGE_CONTENT
val message = MESSAGE_REGULAR.copy(
content = MessageContent.Asset(assetContent)
)
val (arrangement, useCase) = Arrangement()
.arrange {
withCurrentClientIdSuccess(CURRENT_CLIENT_ID)
withSelfConversationIds(SELF_CONVERSATION_ID)
withSendMessageSucceed()
withDeleteMessage(Either.Right(Unit))
withMarkAsDeleted(Either.Right(Unit), EqualsMatcher(message.id), EqualsMatcher(message.conversationId))
withGetMessageById(Either.Right(message), EqualsMatcher(message.id), EqualsMatcher(message.conversationId))
withDeleteAssetLocally(Either.Right(Unit), EqualsMatcher(assetContent.remoteData.assetId))
}

useCase(message.conversationId, message.id).shouldSucceed()

coVerify {
arrangement.assetRepository.deleteAssetLocally(assetContent.remoteData.assetId)
}.wasInvoked(exactly = once)
}

private companion object {
val selfUserId = UserId("selfUserId", "selfUserDomain.sy")
val SELF_CONVERSION_ID = listOf(ConversationId("selfConversationId", "selfConversationDomain.com"))
val SELF_USER_ID = UserId("selfUserId", "selfUserDomain.sy")
val SENDER_USER_ID = UserId("senderUserId", "senderDomain")
val SELF_CONVERSATION_ID = listOf(ConversationId("selfConversationId", "selfConversationDomain.com"))
val CURRENT_CLIENT_ID = ClientId("currentClientId")
val ASSET_CONTENT_REMOTE_DATA = AssetContent.RemoteData(
otrKey = ByteArray(0),
sha256 = ByteArray(16),
assetId = "asset-id",
assetToken = "==some-asset-token",
assetDomain = "some-asset-domain.com",
encryptionAlgorithm = MessageEncryptionAlgorithm.AES_GCM
)
val ASSET_IMAGE_CONTENT = AssetContent(
0L,
"name",
"image/jpg",
AssetContent.AssetMetadata.Image(100, 100),
ASSET_CONTENT_REMOTE_DATA
)
val MESSAGE_REGULAR = Message.Regular(
id = "messageId",
content = MessageContent.Text("text"),
conversationId = ConversationId("conversationId", "conversationDomain"),
date = Instant.DISTANT_FUTURE,
senderUserId = SENDER_USER_ID,
senderClientId = CURRENT_CLIENT_ID,
status = Message.Status.Pending,
editStatus = Message.EditStatus.NotEdited,
isSelfMessage = true
)
}

private class Arrangement :
Expand All @@ -128,7 +169,7 @@ class DeleteEphemeralMessageForSelfUserAsReceiverUseCaseTest {
DeleteEphemeralMessageForSelfUserAsReceiverUseCaseImpl(
messageRepository = messageRepository,
messageSender = messageSender,
selfUserId = selfUserId,
selfUserId = SELF_USER_ID,
selfConversationIdProvider = selfConversationIdProvider,
assetRepository = assetRepository,
currentClientIdProvider = currentClientIdProvider
Expand Down
6 changes: 2 additions & 4 deletions persistence/src/commonMain/db_user/com/wire/kalium/persistence/Messages.sq
View file
Original file line number Diff line number Diff line change
Expand Up @@ -558,11 +558,9 @@ WHERE conversation_id = :conversation_id AND status = 'PENDING';
selectPendingEphemeralMessages:
SELECT * FROM MessageDetailsView
WHERE expireAfterMillis NOT NULL
AND selfDeletionEndDate NOT NULL
AND visibility = "VISIBLE"
AND (
selfDeletionEndDate IS NULL
OR selfDeletionEndDate > STRFTIME('%s', 'now') * 1000 -- Checks if message end date is higher than current time in millis
);
AND selfDeletionEndDate > STRFTIME('%s', 'now') * 1000; -- Checks if message end date is higher than current time in millis

selectAlreadyEndedEphemeralMessages:
SELECT * FROM MessageDetailsView
Expand Down
13 changes: 11 additions & 2 deletions persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/message/MessageDAOTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2031,8 +2031,17 @@ class MessageDAOTest : BaseDatabaseTest() {
status = MessageEntity.Status.SENT,
senderName = userEntity1.name!!,
)
val expectedMessages = listOf(alreadyEndedEphemeralMessage)
val allMessages = expectedMessages + listOf(pendingEphemeralMessage, nonEphemeralMessage)
val notYetStartedEphemeralMessage = newRegularMessageEntity(
"4",
conversationId = conversationEntity1.id,
senderUserId = userEntity1.id,
status = MessageEntity.Status.SENT,
senderName = userEntity1.name!!,
selfDeletionEndDate = null,
expireAfterMs = 1.seconds.inWholeSeconds
)
val expectedMessages = listOf(pendingEphemeralMessage)
val allMessages = expectedMessages + listOf(alreadyEndedEphemeralMessage, nonEphemeralMessage, notYetStartedEphemeralMessage)

messageDAO.insertOrIgnoreMessages(allMessages)

Expand Down

0 comments on commit 2df0159

Please sign in to comment.