chore: asset not found download status [WPB-4989] (#2250)

* chore: asset not found download status * review fixes * test fix
wireapp · Dec 5, 2023 · 0411f52 · 0411f52
1 parent e4246ec
commit 0411f52
Show file tree

Hide file tree

Showing 9 changed files with 73 additions and 47 deletions.
diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/asset/AssetMapper.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/asset/AssetMapper.kt
@@ -50,10 +50,6 @@ interface AssetMapper {
     fun fromAssetEntityToAssetContent(assetContentEntity: MessageEntityContent.Asset): AssetContent
     fun fromProtoAssetMessageToAssetContent(protoAssetMessage: Asset): AssetContent
     fun fromAssetContentToProtoAssetMessage(messageContent: MessageContent.Asset, expectsReadConfirmation: Boolean): Asset
-    fun fromDownloadStatusToDaoModel(downloadStatus: Message.DownloadStatus): MessageEntity.DownloadStatus
-    fun fromDownloadStatusEntityToLogicModel(downloadStatus: MessageEntity.DownloadStatus?): Message.DownloadStatus
-    fun fromUploadStatusToDaoModel(uploadStatus: Message.UploadStatus): MessageEntity.UploadStatus
-    fun fromUploadStatusEntityToLogicModel(uploadStatus: MessageEntity.UploadStatus?): Message.UploadStatus
 }
 
 class AssetMapperImpl(
@@ -116,8 +112,8 @@ class AssetMapperImpl(
                         else -> AES_CBC
                     }
                 ),
-                uploadStatus = fromUploadStatusEntityToLogicModel(assetUploadStatus),
-                downloadStatus = fromDownloadStatusEntityToLogicModel(assetDownloadStatus)
+                uploadStatus = assetUploadStatus.toModel(),
+                downloadStatus = assetDownloadStatus.toModel()
             )
         }
     }
@@ -235,44 +231,46 @@ class AssetMapperImpl(
                 expectsReadConfirmation = expectsReadConfirmation
             )
         }
+}
 
-    override fun fromUploadStatusToDaoModel(uploadStatus: Message.UploadStatus): MessageEntity.UploadStatus {
-        return when (uploadStatus) {
-            Message.UploadStatus.NOT_UPLOADED -> MessageEntity.UploadStatus.NOT_UPLOADED
-            Message.UploadStatus.UPLOAD_IN_PROGRESS -> MessageEntity.UploadStatus.IN_PROGRESS
-            Message.UploadStatus.UPLOADED -> MessageEntity.UploadStatus.UPLOADED
-            Message.UploadStatus.FAILED_UPLOAD -> MessageEntity.UploadStatus.FAILED
-        }
+fun Message.UploadStatus.toDao(): MessageEntity.UploadStatus {
+    return when (this) {
+        Message.UploadStatus.NOT_UPLOADED -> MessageEntity.UploadStatus.NOT_UPLOADED
+        Message.UploadStatus.UPLOAD_IN_PROGRESS -> MessageEntity.UploadStatus.IN_PROGRESS
+        Message.UploadStatus.UPLOADED -> MessageEntity.UploadStatus.UPLOADED
+        Message.UploadStatus.FAILED_UPLOAD -> MessageEntity.UploadStatus.FAILED
     }
+}
 
-    override fun fromUploadStatusEntityToLogicModel(uploadStatus: MessageEntity.UploadStatus?): Message.UploadStatus {
-        return when (uploadStatus) {
-            MessageEntity.UploadStatus.NOT_UPLOADED -> Message.UploadStatus.NOT_UPLOADED
-            MessageEntity.UploadStatus.IN_PROGRESS -> Message.UploadStatus.UPLOAD_IN_PROGRESS
-            MessageEntity.UploadStatus.UPLOADED -> Message.UploadStatus.UPLOADED
-            MessageEntity.UploadStatus.FAILED -> Message.UploadStatus.FAILED_UPLOAD
-            null -> Message.UploadStatus.NOT_UPLOADED
-        }
+fun MessageEntity.UploadStatus?.toModel(): Message.UploadStatus {
+    return when (this) {
+        MessageEntity.UploadStatus.NOT_UPLOADED -> Message.UploadStatus.NOT_UPLOADED
+        MessageEntity.UploadStatus.IN_PROGRESS -> Message.UploadStatus.UPLOAD_IN_PROGRESS
+        MessageEntity.UploadStatus.UPLOADED -> Message.UploadStatus.UPLOADED
+        MessageEntity.UploadStatus.FAILED -> Message.UploadStatus.FAILED_UPLOAD
+        null -> Message.UploadStatus.NOT_UPLOADED
     }
+}
 
-    override fun fromDownloadStatusToDaoModel(downloadStatus: Message.DownloadStatus): MessageEntity.DownloadStatus {
-        return when (downloadStatus) {
-            Message.DownloadStatus.NOT_DOWNLOADED -> MessageEntity.DownloadStatus.NOT_DOWNLOADED
-            Message.DownloadStatus.DOWNLOAD_IN_PROGRESS -> MessageEntity.DownloadStatus.IN_PROGRESS
-            Message.DownloadStatus.SAVED_INTERNALLY -> MessageEntity.DownloadStatus.SAVED_INTERNALLY
-            Message.DownloadStatus.SAVED_EXTERNALLY -> MessageEntity.DownloadStatus.SAVED_EXTERNALLY
-            Message.DownloadStatus.FAILED_DOWNLOAD -> MessageEntity.DownloadStatus.FAILED
-        }
+fun Message.DownloadStatus.toDao(): MessageEntity.DownloadStatus {
+    return when (this) {
+        Message.DownloadStatus.NOT_DOWNLOADED -> MessageEntity.DownloadStatus.NOT_DOWNLOADED
+        Message.DownloadStatus.DOWNLOAD_IN_PROGRESS -> MessageEntity.DownloadStatus.IN_PROGRESS
+        Message.DownloadStatus.SAVED_INTERNALLY -> MessageEntity.DownloadStatus.SAVED_INTERNALLY
+        Message.DownloadStatus.SAVED_EXTERNALLY -> MessageEntity.DownloadStatus.SAVED_EXTERNALLY
+        Message.DownloadStatus.FAILED_DOWNLOAD -> MessageEntity.DownloadStatus.FAILED
+        Message.DownloadStatus.NOT_FOUND -> MessageEntity.DownloadStatus.NOT_FOUND
     }
+}
 
-    override fun fromDownloadStatusEntityToLogicModel(downloadStatus: MessageEntity.DownloadStatus?): Message.DownloadStatus {
-        return when (downloadStatus) {
-            MessageEntity.DownloadStatus.NOT_DOWNLOADED -> Message.DownloadStatus.NOT_DOWNLOADED
-            MessageEntity.DownloadStatus.IN_PROGRESS -> Message.DownloadStatus.DOWNLOAD_IN_PROGRESS
-            MessageEntity.DownloadStatus.SAVED_INTERNALLY -> Message.DownloadStatus.SAVED_INTERNALLY
-            MessageEntity.DownloadStatus.SAVED_EXTERNALLY -> Message.DownloadStatus.SAVED_EXTERNALLY
-            MessageEntity.DownloadStatus.FAILED -> Message.DownloadStatus.FAILED_DOWNLOAD
-            null -> Message.DownloadStatus.NOT_DOWNLOADED
-        }
+fun MessageEntity.DownloadStatus?.toModel(): Message.DownloadStatus {
+    return when (this) {
+        MessageEntity.DownloadStatus.NOT_DOWNLOADED -> Message.DownloadStatus.NOT_DOWNLOADED
+        MessageEntity.DownloadStatus.IN_PROGRESS -> Message.DownloadStatus.DOWNLOAD_IN_PROGRESS
+        MessageEntity.DownloadStatus.SAVED_INTERNALLY -> Message.DownloadStatus.SAVED_INTERNALLY
+        MessageEntity.DownloadStatus.SAVED_EXTERNALLY -> Message.DownloadStatus.SAVED_EXTERNALLY
+        MessageEntity.DownloadStatus.FAILED -> Message.DownloadStatus.FAILED_DOWNLOAD
+        MessageEntity.DownloadStatus.NOT_FOUND -> Message.DownloadStatus.NOT_FOUND
+        null -> Message.DownloadStatus.NOT_DOWNLOADED
     }
 }
diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/message/Message.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/message/Message.kt
@@ -550,7 +550,12 @@ sealed interface Message {
         /**
          * The last attempt at fetching and saving this asset's data failed.
          */
-        FAILED_DOWNLOAD
+        FAILED_DOWNLOAD,
+
+        /**
+         * Asset was not found on the server
+         */
+        NOT_FOUND
     }
 
     enum class Visibility {

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/message/MessageMapper.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/message/MessageMapper.kt
@@ -19,6 +19,7 @@
 package com.wire.kalium.logic.data.message
 
 import com.wire.kalium.logic.data.asset.AssetMapper
+import com.wire.kalium.logic.data.asset.toDao
 import com.wire.kalium.logic.data.conversation.ClientId
 import com.wire.kalium.logic.data.conversation.toDao
 import com.wire.kalium.logic.data.conversation.toModel
@@ -292,8 +293,8 @@ class MessageMapperImpl(
                 assetSizeInBytes = sizeInBytes,
                 assetName = name,
                 assetMimeType = mimeType,
-                assetUploadStatus = assetMapper.fromUploadStatusToDaoModel(uploadStatus),
-                assetDownloadStatus = assetMapper.fromDownloadStatusToDaoModel(downloadStatus),
+                assetUploadStatus = uploadStatus.toDao(),
+                assetDownloadStatus = downloadStatus.toDao(),
                 assetOtrKey = remoteData.otrKey,
                 assetSha256Key = remoteData.sha256,
                 assetId = remoteData.assetId,

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/message/MessageRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/message/MessageRepository.kt
@@ -22,6 +22,7 @@ import com.wire.kalium.logic.CoreFailure
 import com.wire.kalium.logic.NetworkFailure
 import com.wire.kalium.logic.StorageFailure
 import com.wire.kalium.logic.data.asset.AssetMapper
+import com.wire.kalium.logic.data.asset.toDao
 import com.wire.kalium.logic.data.conversation.ClientId
 import com.wire.kalium.logic.data.conversation.Conversation
 import com.wire.kalium.logic.data.conversation.ReceiptModeMapper
@@ -371,7 +372,7 @@ class MessageDataSource(
     ): Either<CoreFailure, Unit> =
         wrapStorageRequest {
             messageDAO.updateAssetUploadStatus(
-                assetMapper.fromUploadStatusToDaoModel(uploadStatus),
+                uploadStatus.toDao(),
                 messageUuid,
                 conversationId.toDao()
             )
@@ -384,7 +385,7 @@ class MessageDataSource(
     ): Either<CoreFailure, Unit> =
         wrapStorageRequest {
             messageDAO.updateAssetDownloadStatus(
-                assetMapper.fromDownloadStatusToDaoModel(downloadStatus),
+                downloadStatus.toDao(),
                 messageUuid,
                 conversationId.toDao()
             )

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/GetMessageAssetUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/GetMessageAssetUseCase.kt
@@ -36,6 +36,8 @@ import com.wire.kalium.logic.data.message.MessageRepository
 import com.wire.kalium.logic.data.user.UserRepository
 import com.wire.kalium.logic.functional.fold
 import com.wire.kalium.logic.kaliumLogger
+import com.wire.kalium.network.exceptions.KaliumException
+import com.wire.kalium.network.exceptions.isNotFoundLabel
 import com.wire.kalium.util.KaliumDispatcher
 import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.CoroutineScope
@@ -116,7 +118,15 @@ internal class GetMessageAssetUseCaseImpl(
                         ).fold({
                             kaliumLogger.e("There was an error downloading asset with id => ${assetMetadata.assetKey.obfuscateId()}")
                             // This should be called if there is an issue while downloading the asset
-                            updateAssetMessageDownloadStatus(Message.DownloadStatus.FAILED_DOWNLOAD, conversationId, messageId)
+                            if (it is NetworkFailure.ServerMiscommunication &&
+                                it.kaliumException is KaliumException.InvalidRequestError
+                                && it.kaliumException.isNotFoundLabel()
+                            ) {
+                                updateAssetMessageDownloadStatus(Message.DownloadStatus.NOT_FOUND, conversationId, messageId)
+                            } else {
+                                updateAssetMessageDownloadStatus(Message.DownloadStatus.FAILED_DOWNLOAD, conversationId, messageId)
+                            }
+
                             when {
                                 it.isInvalidRequestError -> {
                                     assetMetadata.assetKeyDomain?.let { domain ->

diff --git a/...c/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/GetMessageAssetUseCaseTest.kt b/...c/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/GetMessageAssetUseCaseTest.kt
@@ -150,7 +150,7 @@ class GetMessageAssetUseCaseTest {
                     ErrorResponse(
                         404,
                         "asset not found",
-                        "asset-not-found"
+                        "not-found"
                     )
                 )
             )
@@ -170,7 +170,7 @@ class GetMessageAssetUseCaseTest {
 
             verify(arrangement.updateAssetMessageDownloadStatus)
                 .suspendFunction(arrangement.updateAssetMessageDownloadStatus::invoke)
-                .with(matching { it == Message.DownloadStatus.FAILED_DOWNLOAD }, eq(someConversationId), eq(someMessageId))
+                .with(matching { it == Message.DownloadStatus.NOT_FOUND }, eq(someConversationId), eq(someMessageId))
                 .wasInvoked(exactly = once)
 
             verify(arrangement.userRepository)

diff --git a/network/src/commonMain/kotlin/com/wire/kalium/network/exceptions/KaliumException.kt b/network/src/commonMain/kotlin/com/wire/kalium/network/exceptions/KaliumException.kt
@@ -45,6 +45,7 @@ import com.wire.kalium.network.exceptions.NetworkErrorLabel.MLS_CLIENT_MISMATCH
 import com.wire.kalium.network.exceptions.NetworkErrorLabel.MLS_COMMIT_MISSING_REFERENCES
 import com.wire.kalium.network.exceptions.NetworkErrorLabel.MLS_MISSING_GROUP_INFO
 import com.wire.kalium.network.exceptions.NetworkErrorLabel.MLS_STALE_MESSAGE
+import com.wire.kalium.network.exceptions.NetworkErrorLabel.NOT_FOUND
 import com.wire.kalium.network.exceptions.NetworkErrorLabel.NOT_TEAM_MEMBER
 import com.wire.kalium.network.exceptions.NetworkErrorLabel.NO_CONVERSATION
 import com.wire.kalium.network.exceptions.NetworkErrorLabel.NO_CONVERSATION_CODE
@@ -130,6 +131,10 @@ fun KaliumException.InvalidRequestError.isNotFound(): Boolean {
     return errorResponse.code == HttpStatusCode.NotFound.value
 }
 
+fun KaliumException.InvalidRequestError.isNotFoundLabel(): Boolean {
+    return errorResponse.label == NOT_FOUND
+}
+
 fun KaliumException.InvalidRequestError.isDomainBlockedForRegistration(): Boolean {
     return errorResponse.label == DOMAIN_BLOCKED_FOR_REGISTRATION
 }

diff --git a/network/src/commonMain/kotlin/com/wire/kalium/network/exceptions/NetworkErrorLabel.kt b/network/src/commonMain/kotlin/com/wire/kalium/network/exceptions/NetworkErrorLabel.kt
@@ -48,6 +48,7 @@ internal object NetworkErrorLabel {
     const val GUEST_LINKS_DISABLED = "guest-links-disabled"
     const val ACCESS_DENIED = "access-denied"
     const val WRONG_CONVERSATION_PASSWORD = "invalid-conversation-password"
+    const val NOT_FOUND = "not-found"
 
     // Federation
     const val FEDERATION_FAILURE = "federation-remote-error"

diff --git a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/message/MessageEntity.kt b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/message/MessageEntity.kt
@@ -179,7 +179,12 @@ sealed interface MessageEntity {
         /**
          * The last attempt at fetching and saving this asset's data failed.
          */
-        FAILED
+        FAILED,
+
+        /**
+         * Asset was not found on the server
+         */
+        NOT_FOUND
     }
 
     enum class ConfirmationType {