Skip to content

Commit

Permalink
fix: delete assets locally when message is deleted [WPB-9466] (#2925)
Browse files Browse the repository at this point in the history
* fix: delete assets locally when message is deleted [WPB-9466]

* detekt

* do not return failure when removing asset
  • Loading branch information
saleniuk authored and github-actions[bot] committed Aug 2, 2024
1 parent 2829746 commit 4488c1f
Show file tree
Hide file tree
Showing 7 changed files with 258 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ interface AssetRepository {
suspend fun fetchDecodedAsset(assetId: String): Either<CoreFailure, Path>
}

@Suppress("TooManyFunctions")
internal class AssetDataSource(
private val assetApi: AssetApi,
private val assetDao: AssetDAO,
Expand Down Expand Up @@ -367,7 +368,22 @@ internal class AssetDataSource(
.flatMap { deleteAssetLocally(assetId) }

override suspend fun deleteAssetLocally(assetId: String): Either<CoreFailure, Unit> =
wrapStorageRequest { assetDao.deleteAsset(assetId) }
deleteAssetFileLocally(assetId).let {
wrapStorageRequest {
assetDao.deleteAsset(assetId)
}
}

private suspend fun deleteAssetFileLocally(assetId: String) {
wrapStorageRequest {
assetDao.getAssetByKey(assetId).firstOrNull()
}.map {
val filePath = it.dataPath.toPath()
if (kaliumFileSystem.exists(filePath)) {
kaliumFileSystem.delete(path = it.dataPath.toPath(), mustExist = false)
}
}
}
}

private fun buildFileName(name: String, extension: String?): String =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,10 @@ class DebugScope internal constructor(
)

private val deleteEphemeralMessageForSelfUserAsSender: DeleteEphemeralMessageForSelfUserAsSenderUseCaseImpl
get() = DeleteEphemeralMessageForSelfUserAsSenderUseCaseImpl(messageRepository)
get() = DeleteEphemeralMessageForSelfUserAsSenderUseCaseImpl(
messageRepository = messageRepository,
assetRepository = assetRepository,
)

private val ephemeralMessageDeletionHandler =
EphemeralMessageDeletionHandlerImpl(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,6 @@ class MessageScope internal constructor(
kaliumLogger = kaliumLogger,
)

private val deleteEphemeralMessageForSelfUserAsSender: DeleteEphemeralMessageForSelfUserAsSenderUseCaseImpl
get() = DeleteEphemeralMessageForSelfUserAsSenderUseCaseImpl(messageRepository)

val enqueueMessageSelfDeletion: EnqueueMessageSelfDeletionUseCase = EnqueueMessageSelfDeletionUseCaseImpl(
ephemeralMessageDeletionHandler = ephemeralMessageDeletionHandler
)
Expand Down Expand Up @@ -418,6 +415,12 @@ class MessageScope internal constructor(
scope = scope
)

private val deleteEphemeralMessageForSelfUserAsSender: DeleteEphemeralMessageForSelfUserAsSenderUseCaseImpl
get() = DeleteEphemeralMessageForSelfUserAsSenderUseCaseImpl(
messageRepository = messageRepository,
assetRepository = assetRepository,
)

private val deleteEphemeralMessageForSelfUserAsReceiver: DeleteEphemeralMessageForSelfUserAsReceiverUseCaseImpl
get() = DeleteEphemeralMessageForSelfUserAsReceiverUseCaseImpl(
messageRepository = messageRepository,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,23 @@
*/
package com.wire.kalium.logic.feature.message.ephemeral

import com.wire.kalium.logger.KaliumLogger.Companion.ApplicationFlow.ASSETS
import com.wire.kalium.logic.CoreFailure
import com.wire.kalium.logic.data.asset.AssetRepository
import com.wire.kalium.logic.data.id.ConversationId
import com.wire.kalium.logic.data.message.Message
import com.wire.kalium.logic.data.message.MessageContent
import com.wire.kalium.logic.data.message.MessageRepository
import com.wire.kalium.logic.functional.Either
import com.wire.kalium.logic.functional.flatMap
import com.wire.kalium.logic.functional.onFailure
import com.wire.kalium.logic.functional.onSuccess
import com.wire.kalium.logic.kaliumLogger

/**
* When the self user is the sender of the self deletion message, we only mark it as deleted because we are relying on the receiver,
* telling us when to delete the message permanently, that is when the message has expired for one of the conversation members
* of GROUP or ONE_TO_ONE type
* When the self user is the sender of the self deletion message, we only mark it as deleted (but we delete asset files locally),
* because we are relying on the receiver, telling us when to delete the message permanently,
* that is when the message has expired for one of the conversation members of GROUP or ONE_TO_ONE type
* see [com.wire.kalium.logic.feature.message.ephemeral.DeleteEphemeralMessageForSelfUserAsReceiverUseCaseImpl] for details
**/
internal interface DeleteEphemeralMessageForSelfUserAsSenderUseCase {
Expand All @@ -37,9 +45,24 @@ internal interface DeleteEphemeralMessageForSelfUserAsSenderUseCase {
}

internal class DeleteEphemeralMessageForSelfUserAsSenderUseCaseImpl internal constructor(
private val messageRepository: MessageRepository
private val messageRepository: MessageRepository,
private val assetRepository: AssetRepository,
) : DeleteEphemeralMessageForSelfUserAsSenderUseCase {

override suspend fun invoke(conversationId: ConversationId, messageId: String): Either<CoreFailure, Unit> =
messageRepository.markMessageAsDeleted(messageId, conversationId)
messageRepository.getMessageById(conversationId, messageId)
.onSuccess { message ->
deleteMessageAssetLocallyIfExists(message)
}
.flatMap {
messageRepository.markMessageAsDeleted(messageId, conversationId)
}

private suspend fun deleteMessageAssetLocallyIfExists(message: Message) {
(message.content as? MessageContent.Asset)?.value?.remoteData?.let { assetToRemove ->
assetRepository.deleteAssetLocally(assetToRemove.assetId).onFailure {
kaliumLogger.withFeatureId(ASSETS).w("delete message asset failure: $it")
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,7 @@ class AssetRepositoryTest {
val (arrangement, assetRepository) = Arrangement()
.withSuccessDeleteRemotelyResponse()
.withSuccessDeleteLocallyResponse()
.withMockedAssetDaoGetByKeyCall(assetKey, null)
.arrange()

// When
Expand All @@ -560,6 +561,51 @@ class AssetRepositoryTest {

}

@Test
fun givenAssetFileExists_whenDeletingRemotelyAsset_thenFileShouldBeDeleted() = runTest {
// Given
val assetKey = UserAssetId("value1", "domain1")
val assetRawData = "some-dummy-data".toByteArray()
val assetFile = fakeKaliumFileSystem.providePersistentAssetPath(assetKey.toString())

val (_, assetRepository) = Arrangement()
.withSuccessDeleteRemotelyResponse()
.withSuccessDeleteLocallyResponse()
.withRawStoredData(assetRawData, assetFile)
.withMockedAssetDaoGetByKeyCall(assetKey, stubAssetEntity(assetKey.value, assetFile, assetRawData.size.toLong()))
.arrange()

assertEquals(true, fakeKaliumFileSystem.exists(assetFile))

// When
assetRepository.deleteAsset(assetKey.value, assetKey.domain, "asset-token")

// Then
assertEquals(false, fakeKaliumFileSystem.exists(assetFile))
}

@Test
fun givenAssetFileExists_whenDeletingLocallyAsset_thenFileShouldBeDeleted() = runTest {
// Given
val assetKey = UserAssetId("value1", "domain1")
val assetRawData = "some-dummy-data".toByteArray()
val assetFile = fakeKaliumFileSystem.providePersistentAssetPath(assetKey.toString())

val (_, assetRepository) = Arrangement()
.withSuccessDeleteLocallyResponse()
.withRawStoredData(assetRawData, assetFile)
.withMockedAssetDaoGetByKeyCall(assetKey, stubAssetEntity(assetKey.value, assetFile, assetRawData.size.toLong()))
.arrange()

assertEquals(true, fakeKaliumFileSystem.exists(assetFile))

// When
assetRepository.deleteAssetLocally(assetKey.value)

// Then
assertEquals(false, fakeKaliumFileSystem.exists(assetFile))
}

@Test
fun givenValidParams_whenPersistingAsset_thenShouldSucceedWithAPathResponse() = runTest {
// Given
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
/*
* Wire
* Copyright (C) 2024 Wire Swiss GmbH
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see http://www.gnu.org/licenses/.
*/
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.message.MessageEncryptionAlgorithm
import com.wire.kalium.logic.data.user.UserId
import com.wire.kalium.logic.feature.message.ephemeral.DeleteEphemeralMessageForSelfUserAsSenderUseCase
import com.wire.kalium.logic.feature.message.ephemeral.DeleteEphemeralMessageForSelfUserAsSenderUseCaseImpl
import com.wire.kalium.logic.functional.Either
import com.wire.kalium.logic.util.arrangement.MessageSenderArrangement
import com.wire.kalium.logic.util.arrangement.MessageSenderArrangementImpl
import com.wire.kalium.logic.util.arrangement.SelfConversationIdProviderArrangement
import com.wire.kalium.logic.util.arrangement.SelfConversationIdProviderArrangementImpl
import com.wire.kalium.logic.util.arrangement.provider.CurrentClientIdProviderArrangement
import com.wire.kalium.logic.util.arrangement.provider.CurrentClientIdProviderArrangementImpl
import com.wire.kalium.logic.util.arrangement.repository.AssetRepositoryArrangement
import com.wire.kalium.logic.util.arrangement.repository.AssetRepositoryArrangementImpl
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 io.mockative.any
import io.mockative.coVerify
import io.mockative.matchers.EqualsMatcher
import io.mockative.once
import kotlinx.coroutines.test.runTest
import kotlinx.datetime.Instant
import kotlin.test.Test

class DeleteEphemeralMessageForSelfUserAsSenderUseCaseTest {

@Test
fun givenMessage_whenDeleting_thenMarkMessageAsDeleted() = runTest {
val message = MESSAGE_REGULAR
val (arrangement, useCase) = Arrangement()
.arrange {
withMarkAsDeleted(Either.Right(Unit), EqualsMatcher(message.id), EqualsMatcher(message.conversationId))
withGetMessageById(Either.Right(message), EqualsMatcher(message.id), EqualsMatcher(message.conversationId))
}

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

coVerify {
arrangement.messageRepository.markMessageAsDeleted(any(), any())
}.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 {
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 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 = UserId("senderId", "senderDomain"),
senderClientId = CURRENT_CLIENT_ID,
status = Message.Status.Pending,
editStatus = Message.EditStatus.NotEdited,
isSelfMessage = true
)
}

private class Arrangement :
CurrentClientIdProviderArrangement by CurrentClientIdProviderArrangementImpl(),
MessageRepositoryArrangement by MessageRepositoryArrangementImpl(),
MessageSenderArrangement by MessageSenderArrangementImpl(),
SelfConversationIdProviderArrangement by SelfConversationIdProviderArrangementImpl(),
AssetRepositoryArrangement by AssetRepositoryArrangementImpl() {

private val useCase: DeleteEphemeralMessageForSelfUserAsSenderUseCase =
DeleteEphemeralMessageForSelfUserAsSenderUseCaseImpl(
messageRepository = messageRepository,
assetRepository = assetRepository,
)

suspend fun arrange(block: suspend Arrangement.() -> Unit): Pair<Arrangement, DeleteEphemeralMessageForSelfUserAsSenderUseCase> {
block()
return this to useCase
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,38 @@
*/
package com.wire.kalium.logic.util.arrangement.repository

import com.wire.kalium.logic.CoreFailure
import com.wire.kalium.logic.data.asset.AssetRepository
import com.wire.kalium.logic.functional.Either
import io.mockative.Mock
import io.mockative.coEvery
import io.mockative.fake.valueOf
import io.mockative.matchers.AnyMatcher
import io.mockative.matchers.Matcher
import io.mockative.mock

internal interface AssetRepositoryArrangement {
@Mock
val assetRepository: AssetRepository

suspend fun withDeleteAssetLocally(
result: Either<CoreFailure, Unit>,
assetID: Matcher<String> = AnyMatcher(valueOf()),
)
}

internal open class AssetRepositoryArrangementImpl : AssetRepositoryArrangement {
@Mock
override val assetRepository: AssetRepository = mock(AssetRepository::class)

override suspend fun withDeleteAssetLocally(
result: Either<CoreFailure, Unit>,
assetID: Matcher<String>,
) {
coEvery {
assetRepository.deleteAssetLocally(
io.mockative.matches { assetID.matches(it) },
)
}.returns(result)
}
}

0 comments on commit 4488c1f

Please sign in to comment.