Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: delete asset file when removing self-deleting msg [WPB-1807] #3027

Merged
merged 1 commit into from
Sep 25, 2024
Merged

fix: delete asset file when removing self-deleting msg [WPB-1807] #3027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 27 additions & 27 deletions ...ium/logic/feature/message/ephemeral/DeleteEphemeralMessageForSelfUserAsReceiverUseCase.kt
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading