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 for message with reply [WPB-14433] 🍒 #3695

Merged
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
1 change: 1 addition & 0 deletions app/src/main/kotlin/com/wire/android/model/ImageAsset.kt
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ sealed class ImageAsset {
val idKey: String
) : ImageAsset()

@Serializable
sealed class Remote : ImageAsset() {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,8 @@ sealed interface UIMessageContent {
@Serializable
data object IncompleteAssetMessage : UIMessageContent

interface PartialDeliverable {
@Serializable
sealed interface PartialDeliverable {
val deliveryStatus: DeliveryStatusContent
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,25 +39,34 @@ sealed class UIQuotedMessage {
val quotedContent: Content
) : UIQuotedMessage() {

@Serializable
sealed interface Content

@Serializable
data class Text(val value: String) : Content

@Serializable
data class GenericAsset(
val assetName: String?,
val assetMimeType: String
) : Content

@Serializable
data class DisplayableImage(
val displayable: ImageAsset.PrivateAsset
) : Content

@Serializable
data class Location(val locationName: String) : Content

object AudioMessage : Content
@Serializable
data object AudioMessage : Content

object Deleted : Content
object Invalid : Content
@Serializable
data object Deleted : Content

@Serializable
data object Invalid : Content
}
}

Expand Down
6 changes: 3 additions & 3 deletions app/src/main/kotlin/com/wire/android/util/ui/UIText.kt
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import androidx.compose.runtime.Composable
import androidx.compose.ui.res.pluralStringResource
import androidx.compose.ui.res.stringResource
import com.wire.android.appLogger
import com.wire.android.util.AnyPrimitiveAsStringSerializer
import com.wire.kalium.logic.data.message.mention.MessageMention
import com.wire.kalium.util.serialization.AnyPrimitiveValueSerializer
import kotlinx.serialization.Serializable

@Serializable
Expand All @@ -41,14 +41,14 @@ sealed class UIText {
@Serializable
class StringResource(
@StringRes val resId: Int,
vararg val formatArgs: @Serializable(with = AnyPrimitiveValueSerializer::class) Any
vararg val formatArgs: @Serializable(with = AnyPrimitiveAsStringSerializer::class) Any
) : UIText()

@Serializable
class PluralResource(
@PluralsRes val resId: Int,
val count: Int,
vararg val formatArgs: @Serializable(with = AnyPrimitiveValueSerializer::class) Any
vararg val formatArgs: @Serializable(with = AnyPrimitiveAsStringSerializer::class) Any
) : UIText()

@Suppress("SpreadOperator")
Expand Down
7 changes: 6 additions & 1 deletion core/ui-common/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ plugins {
id(libs.plugins.wire.android.library.get().pluginId)
id(libs.plugins.wire.kover.get().pluginId)
alias(libs.plugins.kotlin.serialization)
id(BuildPlugins.junit5)
}

android {
Expand Down Expand Up @@ -35,7 +36,11 @@ dependencies {
implementation(libs.coil.gif)
implementation(libs.coil.compose)

testImplementation(libs.junit4)
testImplementation(libs.junit5.core)
testImplementation(libs.junit5.params)
testImplementation(libs.mockk.core)
testImplementation(libs.kluent.core)
testRuntimeOnly(libs.junit5.engine)
androidTestImplementation(libs.androidx.test.extJunit)
androidTestImplementation(libs.androidx.espresso.core)
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ import androidx.compose.material3.SheetValue
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.saveable.Saver
import androidx.compose.runtime.saveable.rememberSaveable
import androidx.compose.runtime.setValue
import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.platform.LocalSoftwareKeyboardController
Expand Down Expand Up @@ -89,13 +89,14 @@ open class WireModalSheetState<T : Any>(
companion object {
const val DELAY_TO_SHOW_BOTTOM_SHEET_WHEN_KEYBOARD_IS_OPEN = 300L

@Suppress("TooGenericExceptionCaught")
@OptIn(InternalSerializationApi::class)
inline fun <reified T : Any> saver(
density: Density,
softwareKeyboardController: SoftwareKeyboardController?,
noinline onDismissAction: () -> Unit,
scope: CoroutineScope
): Saver<WireModalSheetState<T>, *> = Saver(
): Saver<WireModalSheetState<T>, List<Any>> = Saver(
save = {
when (it.currentValue) {
is WireSheetValue.Hidden -> listOf(false) // hidden
Expand All @@ -109,7 +110,13 @@ open class WireModalSheetState<T : Any>(
listOf(true, SavedType.Regular, value)

T::class.serializerOrNull() != null -> // expanded and with non-Unit value that can be serialized
listOf(true, SavedType.SerializedBundle, Bundlizer.bundle(T::class.serializer(), value))
try {
val serializedBundleValue = Bundlizer.bundle(T::class.serializer(), value)
listOf(true, SavedType.SerializedBundle, serializedBundleValue)
} catch (e: Exception) {
e.printStackTrace()
listOf(false) // hidden because value cannot be serialized properly
}

else -> listOf(false) // hidden because value cannot be saved
}
Expand Down Expand Up @@ -159,14 +166,9 @@ inline fun <reified T : Any> rememberWireModalSheetState(
val softwareKeyboardController = LocalSoftwareKeyboardController.current
val density = LocalDensity.current
val scope = rememberCoroutineScope()
return rememberSaveable(
saver = WireModalSheetState.saver(
density = density,
softwareKeyboardController = softwareKeyboardController,
onDismissAction = onDismissAction,
scope = scope
)
) {
// TODO: we can use rememberSaveable instead of remember to save the state but first we need to make sure that we don't store too much,
// especially for conversations and messages to not keep such data unencrypted anywhere
return remember {
WireModalSheetState(
density = density,
scope = scope,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* 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.android.util

import kotlinx.serialization.KSerializer
import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable
import kotlinx.serialization.descriptors.SerialDescriptor
import kotlinx.serialization.encoding.Decoder
import kotlinx.serialization.encoding.Encoder

object AnyPrimitiveAsStringSerializer : KSerializer<Any> {
override val descriptor: SerialDescriptor = AnyPrimitiveSurrogate.serializer().descriptor
override fun serialize(encoder: Encoder, value: Any) =
encoder.encodeSerializableValue(AnyPrimitiveSurrogate.serializer(), AnyPrimitiveSurrogate(value))
override fun deserialize(decoder: Decoder): Any =
decoder.decodeSerializableValue(AnyPrimitiveSurrogate.serializer()).value
}

@Serializable
@SerialName("AnyPrimitive")
private data class AnyPrimitiveSurrogate(private val kind: AnyPrimitiveKind, private val stringValue: String) {
constructor(value: Any) : this(
kind = when (value) {
is String -> AnyPrimitiveKind.STRING
is Int -> AnyPrimitiveKind.INT
is Long -> AnyPrimitiveKind.LONG
is Float -> AnyPrimitiveKind.FLOAT
is Double -> AnyPrimitiveKind.DOUBLE
is Boolean -> AnyPrimitiveKind.BOOLEAN
else -> throw IllegalArgumentException("Unsupported type: ${value::class}")
},
stringValue = value.toString()
)

val value: Any
get() = when (kind) {
AnyPrimitiveKind.STRING -> stringValue
AnyPrimitiveKind.INT -> stringValue.toInt()
AnyPrimitiveKind.LONG -> stringValue.toLong()
AnyPrimitiveKind.FLOAT -> stringValue.toFloat()
AnyPrimitiveKind.DOUBLE -> stringValue.toDouble()
AnyPrimitiveKind.BOOLEAN -> stringValue.toBoolean()
}
}

private enum class AnyPrimitiveKind { STRING, INT, LONG, FLOAT, DOUBLE, BOOLEAN }
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@
*/
package com.wire.android.ui.common

import org.junit.Test

import org.junit.Assert.*
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Test

/**
* Example local unit test, which will execute on the development machine (host).
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
* 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.android.ui.common.bottomsheet

import androidx.compose.runtime.saveable.SaverScope
import com.wire.android.util.AnyPrimitiveAsStringSerializer
import io.mockk.mockk
import kotlinx.serialization.Serializable
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertInstanceOf

class WireModalSheetStateTest {

@Suppress("LongParameterList")
@Serializable
class SerializableTestModel(
val boolean: Boolean,
val int: Int,
val long: Long,
val float: Float,
val double: Double,
val char: Char,
val string: String,
val nullable: String?,
val list: List<String>,
vararg val any: @Serializable(with = AnyPrimitiveAsStringSerializer::class) Any
) {
override fun equals(other: Any?): Boolean {
if (this === other) return true
if (other !is SerializableTestModel) return false

if (boolean != other.boolean) return false
if (int != other.int) return false
if (long != other.long) return false
if (float != other.float) return false
if (double != other.double) return false
if (char != other.char) return false
if (string != other.string) return false
if (nullable != other.nullable) return false
if (list != other.list) return false
if (!any.contentEquals(other.any)) return false

return true
}

override fun hashCode(): Int {
var result = boolean.hashCode()
result = 31 * result + int
result = 31 * result + long.hashCode()
result = 31 * result + float.hashCode()
result = 31 * result + double.hashCode()
result = 31 * result + char.hashCode()
result = 31 * result + string.hashCode()
result = 31 * result + (nullable?.hashCode() ?: 0)
result = 31 * result + list.hashCode()
result = 31 * result + any.contentHashCode()
return result
}
}

@Test
fun givenSerializableModel_whenSavingState_thenStateIsSavedAndRestoredProperly() {
// given
val model = SerializableTestModel(
boolean = true,
int = 1,
long = 2L,
float = 3.0f,
double = 4.0,
char = 'c',
string = "string",
nullable = null,
list = listOf("a", "b", "c"),
any = arrayOf(1, 2L, 3.0, 4f, true, false, 'c', "string")
)
val sheetValue = WireSheetValue.Expanded(model)
with(WireModalSheetState.saver<SerializableTestModel>(mockk(), mockk(), mockk(), mockk())) {
// when
val saved = SaverScope { true }.save(WireModalSheetState(mockk(), mockk(), mockk(), mockk(), sheetValue))
// then
assertInstanceOf<List<Any>>(saved).let {
val restored = restore(it)
assertInstanceOf<WireSheetValue.Expanded<SerializableTestModel>>(restored?.currentValue).let {
assertEquals(sheetValue.value, it.value)
}
}
}
}
}
Loading