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

fix: crash when saving bottom sheet state [WPB-14433] 🍒 #3683

Merged
merged 3 commits into from
Nov 29, 2024
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
8 changes: 3 additions & 5 deletions app/src/main/kotlin/com/wire/android/mapper/ContactMapper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import com.wire.android.ui.home.conversationslist.model.Membership
import com.wire.android.ui.home.newconversation.model.Contact
import com.wire.android.ui.userprofile.common.UsernameMapper
import com.wire.android.util.EMPTY
import com.wire.android.util.ui.WireSessionImageLoader
import com.wire.kalium.logic.data.publicuser.model.UserSearchDetails
import com.wire.kalium.logic.data.service.ServiceDetails
import com.wire.kalium.logic.data.user.ConnectionState
Expand All @@ -36,7 +35,6 @@ import javax.inject.Inject
class ContactMapper
@Inject constructor(
private val userTypeMapper: UserTypeMapper,
private val wireSessionImageLoader: WireSessionImageLoader,
) {

fun fromOtherUser(otherUser: OtherUser): Contact {
Expand All @@ -48,7 +46,7 @@ class ContactMapper
handle = handle.orEmpty(),
label = UsernameMapper.fromOtherUser(otherUser),
avatarData = UserAvatarData(
asset = previewPicture?.let { ImageAsset.UserAvatarAsset(wireSessionImageLoader, it) },
asset = previewPicture?.let { ImageAsset.UserAvatarAsset(it) },
connectionState = connectionStatus,
nameBasedAvatar = NameBasedAvatar(fullName = name, accentColor = otherUser.accentId)
),
Expand All @@ -67,7 +65,7 @@ class ContactMapper
handle = String.EMPTY,
label = String.EMPTY,
avatarData = UserAvatarData(
asset = previewAssetId?.let { ImageAsset.UserAvatarAsset(wireSessionImageLoader, it) },
asset = previewAssetId?.let { ImageAsset.UserAvatarAsset(it) },
membership = Membership.Service
),
membership = Membership.Service,
Expand All @@ -85,7 +83,7 @@ class ContactMapper
handle = handle.orEmpty(),
label = mapUserHandle(user),
avatarData = UserAvatarData(
asset = previewAssetId?.let { ImageAsset.UserAvatarAsset(wireSessionImageLoader, it) },
asset = previewAssetId?.let { ImageAsset.UserAvatarAsset(it) },
nameBasedAvatar = NameBasedAvatar(fullName = name, accentColor = -1)
),
membership = userTypeMapper.toMembership(type),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import com.wire.android.ui.home.conversationslist.model.BlockState
import com.wire.android.ui.home.conversationslist.model.ConversationInfo
import com.wire.android.ui.home.conversationslist.model.ConversationItem
import com.wire.android.ui.home.conversationslist.showLegalHoldIndicator
import com.wire.android.util.ui.WireSessionImageLoader
import com.wire.kalium.logic.data.conversation.ConversationDetails.Connection
import com.wire.kalium.logic.data.conversation.ConversationDetails.Group
import com.wire.kalium.logic.data.conversation.ConversationDetails.OneOne
Expand All @@ -41,7 +40,6 @@ import com.wire.kalium.logic.data.user.UserAvailabilityStatus

@Suppress("LongMethod")
fun ConversationDetailsWithEvents.toConversationItem(
wireSessionImageLoader: WireSessionImageLoader,
userTypeMapper: UserTypeMapper,
searchQuery: String,
selfUserTeamId: TeamId?
Expand Down Expand Up @@ -74,7 +72,7 @@ fun ConversationDetailsWithEvents.toConversationItem(
is OneOne -> {
ConversationItem.PrivateConversation(
userAvatarData = UserAvatarData(
asset = conversationDetails.otherUser.previewPicture?.let { UserAvatarAsset(wireSessionImageLoader, it) },
asset = conversationDetails.otherUser.previewPicture?.let { UserAvatarAsset(it) },
availabilityStatus = conversationDetails.otherUser.availabilityStatus,
connectionState = conversationDetails.otherUser.connectionStatus,
nameBasedAvatar = NameBasedAvatar(conversationDetails.otherUser.name, conversationDetails.otherUser.accentId)
Expand Down Expand Up @@ -111,7 +109,7 @@ fun ConversationDetailsWithEvents.toConversationItem(
is Connection -> {
ConversationItem.ConnectionConversation(
userAvatarData = UserAvatarData(
asset = conversationDetails.otherUser?.previewPicture?.let { UserAvatarAsset(wireSessionImageLoader, it) },
asset = conversationDetails.otherUser?.previewPicture?.let { UserAvatarAsset(it) },
availabilityStatus = conversationDetails.otherUser?.availabilityStatus ?: UserAvailabilityStatus.NONE,
nameBasedAvatar = NameBasedAvatar(conversationDetails.otherUser?.name, conversationDetails.otherUser?.accentId ?: -1)
),
Expand Down
5 changes: 1 addition & 4 deletions app/src/main/kotlin/com/wire/android/mapper/MessageMapper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import com.wire.android.ui.home.conversationslist.model.Membership
import com.wire.android.ui.theme.Accent
import com.wire.android.util.time.ISOFormatter
import com.wire.android.util.ui.UIText
import com.wire.android.util.ui.WireSessionImageLoader
import com.wire.kalium.logic.data.message.DeliveryStatus
import com.wire.kalium.logic.data.message.Message
import com.wire.kalium.logic.data.message.MessageContent
Expand All @@ -52,8 +51,6 @@ class MessageMapper @Inject constructor(
private val userTypeMapper: UserTypeMapper,
private val messageContentMapper: MessageContentMapper,
private val isoFormatter: ISOFormatter,
// TODO(qol): a message mapper should not depend on a UI related component
private val wireSessionImageLoader: WireSessionImageLoader
) {

fun memberIdList(messages: List<Message>): List<UserId> = messages.flatMap { message ->
Expand Down Expand Up @@ -200,7 +197,7 @@ class MessageMapper @Inject constructor(
}

private fun getUserAvatarData(sender: User?) = UserAvatarData(
asset = sender?.previewAsset(wireSessionImageLoader),
asset = sender?.previewAsset(),
availabilityStatus = sender?.availabilityStatus ?: UserAvailabilityStatus.NONE,
membership = sender?.userType?.let { userTypeMapper.toMembership(it) } ?: Membership.None,
connectionState = getConnectionState(sender),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,15 @@ package com.wire.android.mapper

import com.wire.android.ui.home.conversations.avatar
import com.wire.android.ui.userprofile.self.model.OtherAccount
import com.wire.android.util.ui.WireSessionImageLoader
import com.wire.kalium.logic.data.team.Team
import com.wire.kalium.logic.data.user.SelfUser
import javax.inject.Inject

class OtherAccountMapper @Inject constructor(
private val wireSessionImageLoader: WireSessionImageLoader
) {
class OtherAccountMapper @Inject constructor() {
fun toOtherAccount(selfUser: SelfUser, team: Team?): OtherAccount = OtherAccount(
id = selfUser.id,
fullName = selfUser.name ?: "",
avatarData = selfUser.avatar(wireSessionImageLoader, selfUser.connectionStatus),
avatarData = selfUser.avatar(selfUser.connectionStatus),
teamName = team?.name
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import com.wire.android.ui.home.conversations.model.UIMessageContent
import com.wire.android.ui.home.conversations.model.UIQuotedMessage
import com.wire.android.util.time.ISOFormatter
import com.wire.android.util.ui.UIText
import com.wire.android.util.ui.WireSessionImageLoader
import com.wire.kalium.logic.data.asset.AttachmentType
import com.wire.kalium.logic.data.asset.isDisplayableImageMimeType
import com.wire.kalium.logic.data.id.ConversationId
Expand All @@ -50,7 +49,6 @@ import javax.inject.Inject
@Suppress("TooManyFunctions")
class RegularMessageMapper @Inject constructor(
private val messageResourceProvider: MessageResourceProvider,
private val wireSessionImageLoader: WireSessionImageLoader,
private val isoFormatter: ISOFormatter,
) {

Expand Down Expand Up @@ -207,7 +205,6 @@ class RegularMessageMapper @Inject constructor(
is MessageContent.QuotedMessageDetails.Asset -> when (AttachmentType.fromMimeTypeString(quotedContent.assetMimeType)) {
AttachmentType.IMAGE -> UIQuotedMessage.UIQuotedData.DisplayableImage(
ImageAsset.PrivateAsset(
wireSessionImageLoader,
conversationId,
it.messageId,
it.isQuotingSelfUser
Expand Down Expand Up @@ -249,7 +246,6 @@ class RegularMessageMapper @Inject constructor(
UIMessageContent.ImageMessage(
assetId = AssetId(remoteData.assetId, remoteData.assetDomain.orEmpty()),
asset = ImageAsset.PrivateAsset(
wireSessionImageLoader,
message.conversationId,
message.id,
sender is SelfUser
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ class SystemMessageContentMapper @Inject constructor(

private fun mapTeamMemberRemovedMessage(
content: MessageContent.TeamMemberRemoved
): UIMessageContent.SystemMessage = UIMessageContent.SystemMessage.TeamMemberRemoved_Legacy(content)
): UIMessageContent.SystemMessage = UIMessageContent.SystemMessage.TeamMemberRemoved_Legacy(content.userName)

private fun mapConversationRenamedMessage(
senderUserId: UserId,
Expand All @@ -197,7 +197,7 @@ class SystemMessageContentMapper @Inject constructor(
user = sender,
type = SelfNameType.ResourceTitleCase
)
return UIMessageContent.SystemMessage.RenamedConversation(authorName, content)
return UIMessageContent.SystemMessage.RenamedConversation(authorName, content.conversationName)
}

fun mapMemberChangeMessage(
Expand Down Expand Up @@ -271,7 +271,7 @@ class SystemMessageContentMapper @Inject constructor(
UIMessageContent.SystemMessage.HistoryLost

private fun mapMLSWrongEpochWarning(): UIMessageContent.SystemMessage =
UIMessageContent.SystemMessage.MLSWrongEpochWarning()
UIMessageContent.SystemMessage.MLSWrongEpochWarning

private fun mapConversationHistoryListProtocolChanged(): UIMessageContent.SystemMessage =
UIMessageContent.SystemMessage.HistoryLostProtocolChanged
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,10 @@ package com.wire.android.mapper

import com.wire.android.model.ImageAsset
import com.wire.android.ui.calling.model.UICallParticipant
import com.wire.android.util.ui.WireSessionImageLoader
import com.wire.kalium.logic.data.call.Participant
import javax.inject.Inject

class UICallParticipantMapper @Inject constructor(
private val wireSessionImageLoader: WireSessionImageLoader,
private val userTypeMapper: UserTypeMapper,
) {
fun toUICallParticipant(participant: Participant) = UICallParticipant(
Expand All @@ -36,7 +34,7 @@ class UICallParticipantMapper @Inject constructor(
isSpeaking = participant.isSpeaking,
isCameraOn = participant.isCameraOn,
isSharingScreen = participant.isSharingScreen,
avatar = participant.avatarAssetId?.let { ImageAsset.UserAvatarAsset(wireSessionImageLoader, it) },
avatar = participant.avatarAssetId?.let { ImageAsset.UserAvatarAsset(it) },
membership = userTypeMapper.toMembership(participant.userType),
hasEstablishedAudio = participant.hasEstablishedAudio,
accentId = participant.accentId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ package com.wire.android.mapper
import com.wire.android.ui.home.conversations.avatar
import com.wire.android.ui.home.conversations.details.participants.model.UIParticipant
import com.wire.android.ui.home.conversations.previewAsset
import com.wire.android.util.ui.WireSessionImageLoader
import com.wire.kalium.logic.data.message.UserSummary
import com.wire.kalium.logic.data.message.reaction.MessageReaction
import com.wire.kalium.logic.data.message.receipt.DetailedReceipt
Expand All @@ -33,7 +32,6 @@ import javax.inject.Inject

class UIParticipantMapper @Inject constructor(
private val userTypeMapper: UserTypeMapper,
private val wireSessionImageLoader: WireSessionImageLoader
) {
fun toUIParticipant(user: User, isMLSVerified: Boolean = false): UIParticipant = with(user) {
val (userType, connectionState, unavailable) = when (this) {
Expand All @@ -45,7 +43,7 @@ class UIParticipantMapper @Inject constructor(
id = id,
name = name.orEmpty(),
handle = handle.orEmpty(),
avatarData = avatar(wireSessionImageLoader, connectionState),
avatarData = avatar(connectionState),
isSelf = user is SelfUser,
isService = userType == UserType.SERVICE,
membership = userTypeMapper.toMembership(userType),
Expand All @@ -67,7 +65,7 @@ class UIParticipantMapper @Inject constructor(
id = userSummary.userId,
name = userSummary.userName.orEmpty(),
handle = userSummary.userHandle.orEmpty(),
avatarData = userSummary.previewAsset(wireSessionImageLoader),
avatarData = userSummary.previewAsset(),
membership = userTypeMapper.toMembership(userSummary.userType),
unavailable = !userSummary.isUserDeleted && userSummary.userName.orEmpty().isEmpty(),
isDeleted = userSummary.isUserDeleted,
Expand All @@ -83,7 +81,7 @@ class UIParticipantMapper @Inject constructor(
id = userSummary.userId,
name = userSummary.userName.orEmpty(),
handle = userSummary.userHandle.orEmpty(),
avatarData = userSummary.previewAsset(wireSessionImageLoader),
avatarData = userSummary.previewAsset(),
membership = userTypeMapper.toMembership(userSummary.userType),
unavailable = !userSummary.isUserDeleted && userSummary.userName.orEmpty().isEmpty(),
isDeleted = userSummary.isUserDeleted,
Expand All @@ -100,7 +98,7 @@ class UIParticipantMapper @Inject constructor(
id = userSummary.userId,
name = userSummary.userName.orEmpty(),
handle = userSummary.userHandle.orEmpty(),
avatarData = previewAsset(wireSessionImageLoader),
avatarData = previewAsset(),
membership = userTypeMapper.toMembership(userSummary.userType),
unavailable = !userSummary.isUserDeleted && userSummary.userName.orEmpty().isEmpty(),
isDeleted = userSummary.isUserDeleted,
Expand Down
54 changes: 42 additions & 12 deletions app/src/main/kotlin/com/wire/android/model/ImageAsset.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,31 +18,47 @@

package com.wire.android.model

import androidx.appcompat.app.AppCompatActivity
import androidx.compose.runtime.Composable
import androidx.compose.runtime.Stable
import androidx.compose.ui.platform.LocalInspectionMode
import androidx.compose.ui.res.painterResource
import androidx.hilt.navigation.compose.hiltViewModel
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewmodel.compose.LocalViewModelStoreOwner
import com.wire.android.R
import com.wire.android.ui.LocalActivity
import com.wire.android.util.ui.WireSessionImageLoader
import com.wire.kalium.logic.data.id.ConversationId
import com.wire.kalium.logic.data.id.QualifiedIdMapper
import com.wire.kalium.logic.data.user.UserAssetId
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.serialization.KSerializer
import kotlinx.serialization.Serializable
import kotlinx.serialization.descriptors.PrimitiveKind
import kotlinx.serialization.descriptors.PrimitiveSerialDescriptor
import kotlinx.serialization.encoding.Decoder
import kotlinx.serialization.encoding.Encoder
import okio.Path
import okio.Path.Companion.toPath
import javax.inject.Inject

@Stable
@Serializable
sealed class ImageAsset {

/**
* Represents an image asset that is stored locally on the device, and it isn't necessarily bounded to any specific conversation or
* message, i.e. some preview images that the user selected from local device gallery.
*/
@Stable
@Serializable
data class Local(
val dataPath: Path,
val dataPath: @Serializable(with = PathAsStringSerializer::class) Path,
val idKey: String
) : ImageAsset()

sealed class Remote(private val imageLoader: WireSessionImageLoader) : ImageAsset() {
sealed class Remote : ImageAsset() {

/**
* Value that uniquely identifies this Asset,
Expand All @@ -56,39 +72,53 @@ sealed class ImageAsset {
withCrossfadeAnimation: Boolean = false
) = when {
LocalInspectionMode.current -> painterResource(id = R.drawable.ic_welcome_1)
else -> imageLoader.paint(asset = this, fallbackData = fallbackData, withCrossfadeAnimation = withCrossfadeAnimation)
else -> {
hiltViewModel<RemoteAssetImageViewModel>(
// limit the scope of the ViewModel to the current activity so that there's one image loader instance for the Activity
viewModelStoreOwner = checkNotNull(LocalActivity.current as? AppCompatActivity ?: LocalViewModelStoreOwner.current) {
"No ViewModelStoreOwner was provided via LocalViewModelStoreOwner"
},
key = "remote_asset_image_loader"
).imageLoader.paint(asset = this, fallbackData = fallbackData, withCrossfadeAnimation = withCrossfadeAnimation)
}
}
}

@Stable
@Serializable
data class UserAvatarAsset(
private val imageLoader: WireSessionImageLoader,
val userAssetId: UserAssetId
) : Remote(imageLoader) {
) : Remote() {
override val uniqueKey: String
get() = userAssetId.toString()
}

@Stable
@Serializable
data class PrivateAsset(
private val imageLoader: WireSessionImageLoader,
val conversationId: ConversationId,
val messageId: String,
val isSelfAsset: Boolean,
val isEphemeral: Boolean = false
) : Remote(imageLoader) {
) : Remote() {
override fun toString(): String = "$conversationId:$messageId:$isSelfAsset:$isEphemeral"
override val uniqueKey: String
get() = toString()
}
}

fun String.parseIntoPrivateImageAsset(
imageLoader: WireSessionImageLoader,
qualifiedIdMapper: QualifiedIdMapper,
): ImageAsset.PrivateAsset {
fun String.parseIntoPrivateImageAsset(qualifiedIdMapper: QualifiedIdMapper): ImageAsset.PrivateAsset {
val (conversationIdString, messageId, isSelfAsset, isEphemeral) = split(":")
val conversationIdParam = qualifiedIdMapper.fromStringToQualifiedID(conversationIdString)

return ImageAsset.PrivateAsset(imageLoader, conversationIdParam, messageId, isSelfAsset.toBoolean(), isEphemeral.toBoolean())
return ImageAsset.PrivateAsset(conversationIdParam, messageId, isSelfAsset.toBoolean(), isEphemeral.toBoolean())
}

object PathAsStringSerializer : KSerializer<Path> {
override val descriptor = PrimitiveSerialDescriptor("Path", PrimitiveKind.STRING)
override fun serialize(encoder: Encoder, value: Path) = encoder.encodeString(value.toString())
override fun deserialize(decoder: Decoder): Path = decoder.decodeString().toPath(normalize = true)
}

@HiltViewModel
class RemoteAssetImageViewModel @Inject constructor(val imageLoader: WireSessionImageLoader) : ViewModel()
3 changes: 3 additions & 0 deletions app/src/main/kotlin/com/wire/android/model/UserAvatarData.kt
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ import com.wire.android.ui.home.conversationslist.model.Membership
import com.wire.android.util.EMPTY
import com.wire.kalium.logic.data.user.ConnectionState
import com.wire.kalium.logic.data.user.UserAvailabilityStatus
import kotlinx.serialization.Serializable

@Stable
@Serializable
data class UserAvatarData(
val asset: ImageAsset.UserAvatarAsset? = null,
val availabilityStatus: UserAvailabilityStatus = UserAvailabilityStatus.NONE,
Expand All @@ -50,6 +52,7 @@ data class UserAvatarData(
/**
* Holder that can be used to generate an avatar based on the user's full name initials and accent color.
*/
@Serializable
data class NameBasedAvatar(val fullName: String?, val accentColor: Int) {
val initials: String
get() {
Expand Down
Loading
Loading