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.

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

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

Merged
merged 6 commits into from
Nov 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -116,8 +112,8 @@ class AssetMapperImpl(
else -> AES_CBC
}
),
uploadStatus = fromUploadStatusEntityToLogicModel(assetUploadStatus),
downloadStatus = fromDownloadStatusEntityToLogicModel(assetDownloadStatus)
uploadStatus = assetUploadStatus.toModel(),
downloadStatus = assetDownloadStatus.toModel()
)
}
}
Expand Down Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -371,7 +372,7 @@ class MessageDataSource(
): Either<CoreFailure, Unit> =
wrapStorageRequest {
messageDAO.updateAssetUploadStatus(
assetMapper.fromUploadStatusToDaoModel(uploadStatus),
uploadStatus.toDao(),
messageUuid,
conversationId.toDao()
)
Expand All @@ -384,7 +385,7 @@ class MessageDataSource(
): Either<CoreFailure, Unit> =
wrapStorageRequest {
messageDAO.updateAssetDownloadStatus(
assetMapper.fromDownloadStatusToDaoModel(downloadStatus),
downloadStatus.toDao(),
messageUuid,
conversationId.toDao()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a CoreFailure.isInvalidRequestError helper, we might use I think

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 ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ class GetMessageAssetUseCaseTest {
ErrorResponse(
404,
"asset not found",
"asset-not-found"
"not-found"
)
)
)
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading