From 2df0159c55d34a03733f924723111eaf800f3116 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= Date: Tue, 24 Sep 2024 13:49:30 +0200 Subject: [PATCH] fix: delete asset file when removing self-deleting msg [WPB-1807] --- ...eralMessageForSelfUserAsReceiverUseCase.kt | 54 +++++------ .../EphemeralMessageDeletionHandler.kt | 41 ++++----- .../ephemeral/SelfDeletionEventLogger.kt | 1 + ...MessageForSelfUserAsReceiverUseCaseTest.kt | 91 ++++++++++++++----- .../com/wire/kalium/persistence/Messages.sq | 6 +- .../persistence/dao/message/MessageDAOTest.kt | 13 ++- 6 files changed, 125 insertions(+), 81 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/message/ephemeral/DeleteEphemeralMessageForSelfUserAsReceiverUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/message/ephemeral/DeleteEphemeralMessageForSelfUserAsReceiverUseCase.kt index a83b73e065e..1105f1a24af 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/message/ephemeral/DeleteEphemeralMessageForSelfUserAsReceiverUseCase.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/message/ephemeral/DeleteEphemeralMessageForSelfUserAsReceiverUseCase.kt @@ -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 = - 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, @@ -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") } } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/message/ephemeral/EphemeralMessageDeletionHandler.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/message/ephemeral/EphemeralMessageDeletionHandler.kt index 13fa1c578da..515895fadd3 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/message/ephemeral/EphemeralMessageDeletionHandler.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/message/ephemeral/EphemeralMessageDeletionHandler.kt @@ -62,43 +62,38 @@ internal class EphemeralMessageDeletionHandlerImpl( private val ongoingSelfDeletionMessagesMutex = Mutex() private val ongoingSelfDeletionMessages = mutableMapOf, 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) } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/message/ephemeral/SelfDeletionEventLogger.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/message/ephemeral/SelfDeletionEventLogger.kt index fdfe5ba25dc..c0bb1d77772 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/message/ephemeral/SelfDeletionEventLogger.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/message/ephemeral/SelfDeletionEventLogger.kt @@ -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()) diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/message/ephemeral/DeleteEphemeralMessageForSelfUserAsReceiverUseCaseTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/message/ephemeral/DeleteEphemeralMessageForSelfUserAsReceiverUseCaseTest.kt index bad8ecd2941..a3eb57969ef 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/message/ephemeral/DeleteEphemeralMessageForSelfUserAsReceiverUseCaseTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/message/ephemeral/DeleteEphemeralMessageForSelfUserAsReceiverUseCaseTest.kt @@ -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 @@ -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 @@ -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)) @@ -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 { @@ -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) @@ -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 : @@ -128,7 +169,7 @@ class DeleteEphemeralMessageForSelfUserAsReceiverUseCaseTest { DeleteEphemeralMessageForSelfUserAsReceiverUseCaseImpl( messageRepository = messageRepository, messageSender = messageSender, - selfUserId = selfUserId, + selfUserId = SELF_USER_ID, selfConversationIdProvider = selfConversationIdProvider, assetRepository = assetRepository, currentClientIdProvider = currentClientIdProvider diff --git a/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Messages.sq b/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Messages.sq index 82764a8a468..347892e9bd2 100644 --- a/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Messages.sq +++ b/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Messages.sq @@ -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 diff --git a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/message/MessageDAOTest.kt b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/message/MessageDAOTest.kt index ae89c137b02..e5e825570c2 100644 --- a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/message/MessageDAOTest.kt +++ b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/dao/message/MessageDAOTest.kt @@ -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)