From 0411f5230e1870bd1844d236e06065985c9a4020 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BBerko?= Date: Fri, 24 Nov 2023 13:55:09 +0800 Subject: [PATCH] chore: asset not found download status [WPB-4989] (#2250) * chore: asset not found download status * review fixes * test fix --- .../kalium/logic/data/asset/AssetMapper.kt | 74 +++++++++---------- .../wire/kalium/logic/data/message/Message.kt | 7 +- .../logic/data/message/MessageMapper.kt | 5 +- .../logic/data/message/MessageRepository.kt | 5 +- .../feature/asset/GetMessageAssetUseCase.kt | 12 ++- .../asset/GetMessageAssetUseCaseTest.kt | 4 +- .../network/exceptions/KaliumException.kt | 5 ++ .../network/exceptions/NetworkErrorLabel.kt | 1 + .../persistence/dao/message/MessageEntity.kt | 7 +- 9 files changed, 73 insertions(+), 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 index d25b09dad07..7fb36a2f008 100644 --- 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 index ba31731992f..287b614e4e7 100644 --- 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 index 58215a7c6be..9e25739ab35 100644 --- 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 index f7f4a46d855..779d4c5db5b 100644 --- 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 = wrapStorageRequest { messageDAO.updateAssetUploadStatus( - assetMapper.fromUploadStatusToDaoModel(uploadStatus), + uploadStatus.toDao(), messageUuid, conversationId.toDao() ) @@ -384,7 +385,7 @@ class MessageDataSource( ): Either = 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 index 1cb7f4ad33a..e27a55dd751 100644 --- 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/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/GetMessageAssetUseCaseTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/GetMessageAssetUseCaseTest.kt index 30b4a98ba68..df71bc399cc 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/GetMessageAssetUseCaseTest.kt +++ b/logic/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 index df48be0e151..6d71f50f446 100644 --- 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 index 8159370cbef..64dfd5067df 100644 --- 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 index 3bee6c6f42e..3e37afeadd5 100644 --- 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 {