Skip to content

Commit

Permalink
fix: crash when saving bottom sheet state for message with reply [WPB…
Browse files Browse the repository at this point in the history
…-14433] (#3689)
  • Loading branch information
saleniuk authored and github-actions[bot] committed Dec 2, 2024
1 parent d5b89ca commit 8e65480
Show file tree
Hide file tree
Showing 9 changed files with 206 additions and 22 deletions.
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)
}
}
}
}
}

0 comments on commit 8e65480

Please sign in to comment.