From e44b401bd5baed906e43fde01f566275fd41e2c3 Mon Sep 17 00:00:00 2001 From: Andrew Date: Sat, 15 Jun 2024 00:36:10 +0930 Subject: [PATCH] Fix New Message ONS request timeout --- .../newmessage/NewMessageFragment.kt | 37 +++++++++++-------- .../newmessage/NewMessageViewModel.kt | 30 ++++++++++----- .../securesms/ui/components/Text.kt | 15 +++++--- .../libsignal/utilities/PromiseUtilities.kt | 19 ++++++++++ 4 files changed, 71 insertions(+), 30 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/newmessage/NewMessageFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/newmessage/NewMessageFragment.kt index e8bb24403d..12db4883f1 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/newmessage/NewMessageFragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/newmessage/NewMessageFragment.kt @@ -5,6 +5,12 @@ import android.os.Bundle import android.view.LayoutInflater import android.view.View import android.view.ViewGroup +import androidx.compose.animation.AnimatedVisibility +import androidx.compose.animation.animateContentSize +import androidx.compose.animation.expandIn +import androidx.compose.animation.scaleIn +import androidx.compose.animation.scaleOut +import androidx.compose.animation.shrinkOut import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.background import androidx.compose.foundation.layout.Arrangement @@ -41,10 +47,10 @@ import org.thoughtcrime.securesms.conversation.start.NewConversationDelegate import org.thoughtcrime.securesms.conversation.v2.ConversationActivityV2 import org.thoughtcrime.securesms.dependencies.DatabaseComponent import org.thoughtcrime.securesms.showOpenUrlDialog -import org.thoughtcrime.securesms.ui.LoadingArcOr -import org.thoughtcrime.securesms.ui.LocalDimensions -import org.thoughtcrime.securesms.ui.LocalColors import org.thoughtcrime.securesms.ui.Colors +import org.thoughtcrime.securesms.ui.LoadingArcOr +import org.thoughtcrime.securesms.ui.LocalColors +import org.thoughtcrime.securesms.ui.LocalDimensions import org.thoughtcrime.securesms.ui.PreviewTheme import org.thoughtcrime.securesms.ui.SessionColorsParameterProvider import org.thoughtcrime.securesms.ui.components.AppBar @@ -75,7 +81,7 @@ class NewMessageFragment : Fragment() { override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle? - ): View = requireContext().createThemedComposeView { + ): View = createThemedComposeView { val uiState by viewModel.state.collectAsState(State()) NewMessage( uiState, @@ -157,17 +163,17 @@ fun EnterAccountId( onContinue = callbacks::onContinue, error = state.error?.string(), ) - if (state.error == null) { - BorderlessButtonWithIcon( - text = stringResource(R.string.messageNewDescription), - iconRes = R.drawable.ic_circle_question_mark, - contentColor = LocalColors.current.textSecondary, - modifier = Modifier - .contentDescription(R.string.AccessibilityId_help_desk_link) - .fillMaxWidth() - .padding(horizontal = LocalDimensions.current.marginMedium), - ) { onHelp() } - } + + BorderlessButtonWithIcon( + text = stringResource(R.string.messageNewDescription), + iconRes = R.drawable.ic_circle_question_mark, + contentColor = LocalColors.current.textSecondary, + modifier = Modifier + .animateContentSize() + .contentDescription(R.string.AccessibilityId_help_desk_link) +// .padding(horizontal = LocalDimensions.current.marginMedium) + .fillMaxWidth(), + ) { onHelp() } SlimOutlineButton( modifier = Modifier @@ -175,6 +181,7 @@ fun EnterAccountId( .padding(horizontal = LocalDimensions.current.marginLarge) .fillMaxWidth() .contentDescription(R.string.next), + color = LocalColors.current.primary, enabled = state.isNextButtonEnabled, onClick = { callbacks.onContinue() } ) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/newmessage/NewMessageViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/newmessage/NewMessageViewModel.kt index 6c919fbf37..8ec9dc1ce2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/newmessage/NewMessageViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/newmessage/NewMessageViewModel.kt @@ -5,21 +5,25 @@ import androidx.lifecycle.AndroidViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.Job +import kotlinx.coroutines.TimeoutCancellationException import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.receiveAsFlow +import kotlinx.coroutines.flow.timeout import kotlinx.coroutines.flow.update -import kotlinx.coroutines.isActive import kotlinx.coroutines.launch -import kotlinx.coroutines.withTimeout import network.loki.messenger.R import org.session.libsession.snode.SnodeAPI import org.session.libsignal.utilities.PublicKeyValidation +import org.session.libsignal.utilities.asFlow import org.thoughtcrime.securesms.ui.GetString import javax.inject.Inject +import kotlin.coroutines.cancellation.CancellationException import kotlin.time.Duration.Companion.seconds @HiltViewModel @@ -57,6 +61,7 @@ class NewMessageViewModel @Inject constructor( } } + @OptIn(FlowPreview::class) private fun createPrivateChatIfPossible(onsNameOrPublicKey: String) { if (loadOnsJob?.isActive == true) return @@ -72,15 +77,20 @@ class NewMessageViewModel @Inject constructor( loadOnsJob = viewModelScope.launch(Dispatchers.IO) { try { - withTimeout(5.seconds) { - SnodeAPI.getSessionID(onsNameOrPublicKey).get() - } - if (isActive) { - _state.update { it.copy(loading = false) } - onPublicKey(onsNameOrPublicKey) - } + // TODO move timeout to SnodeAPI#getSessionID + SnodeAPI.getSessionID(onsNameOrPublicKey).asFlow() + .timeout(30.seconds) + .collectLatest { + _state.update { it.copy(loading = false) } + onPublicKey(onsNameOrPublicKey) + } } catch (e: Exception) { - if (isActive) _state.update { it.copy(loading = false, error = GetString(e) { it.toMessage() }) } + // JobCancellationException is thrown if we cancel the job + // but it is internal, so this excludes other subclasses of CancellationException + // than TimeoutCancellationException + if (e is TimeoutCancellationException == e is CancellationException) { + _state.update { it.copy(loading = false, error = GetString(e) { it.toMessage() }) } + } } } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/ui/components/Text.kt b/app/src/main/java/org/thoughtcrime/securesms/ui/components/Text.kt index 7777104b90..88935e8dfc 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/ui/components/Text.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/ui/components/Text.kt @@ -1,8 +1,11 @@ package org.thoughtcrime.securesms.ui.components import androidx.annotation.DrawableRes +import androidx.compose.animation.animateContentSize import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.foundation.text.InlineTextContent @@ -24,7 +27,6 @@ import androidx.compose.ui.unit.TextUnit import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp import org.thoughtcrime.securesms.ui.LocalColors -import org.thoughtcrime.securesms.ui.LocalDimensions import org.thoughtcrime.securesms.ui.base import org.thoughtcrime.securesms.ui.baseBold import org.thoughtcrime.securesms.ui.outlinedTextFieldColors @@ -38,7 +40,7 @@ fun SessionOutlinedTextField( onContinue: () -> Unit, error: String? = null ) { - Column(modifier = modifier) { + Column(modifier = modifier.animateContentSize()) { OutlinedTextField( value = text, modifier = Modifier.fillMaxWidth(), @@ -62,9 +64,10 @@ fun SessionOutlinedTextField( shape = RoundedCornerShape(12.dp) ) error?.let { + Spacer(modifier = Modifier.height(14.dp)) Text( it, - modifier = Modifier.padding(top = LocalDimensions.current.marginExtraExtraSmall), + modifier = Modifier.fillMaxWidth(), textAlign = TextAlign.Center, style = baseBold, color = LocalColors.current.danger @@ -79,7 +82,7 @@ fun AnnotatedTextWithIcon( @DrawableRes iconRes: Int, modifier: Modifier = Modifier, style: TextStyle = base, - iconTint: Color = Color.Unspecified, + color: Color = Color.Unspecified, iconSize: TextUnit = 12.sp ) { val myId = "inlineContent" @@ -102,7 +105,7 @@ fun AnnotatedTextWithIcon( painter = painterResource(id = iconRes), contentDescription = null, modifier = Modifier.padding(1.dp), - tint = iconTint + tint = color ) } ) @@ -112,6 +115,8 @@ fun AnnotatedTextWithIcon( text = annotated, modifier = modifier.fillMaxWidth(), style = style, + color = color, + textAlign = TextAlign.Center, inlineContent = inlineContent ) } \ No newline at end of file diff --git a/libsignal/src/main/java/org/session/libsignal/utilities/PromiseUtilities.kt b/libsignal/src/main/java/org/session/libsignal/utilities/PromiseUtilities.kt index fdf8f107b9..b2e187f134 100644 --- a/libsignal/src/main/java/org/session/libsignal/utilities/PromiseUtilities.kt +++ b/libsignal/src/main/java/org/session/libsignal/utilities/PromiseUtilities.kt @@ -1,6 +1,9 @@ @file:JvmName("PromiseUtilities") package org.session.libsignal.utilities +import kotlinx.coroutines.channels.awaitClose +import kotlinx.coroutines.flow.callbackFlow +import kotlinx.coroutines.isActive import nl.komponents.kovenant.Promise import nl.komponents.kovenant.deferred import nl.komponents.kovenant.functional.map @@ -67,3 +70,19 @@ infix fun Promise.sideEffect( callback(it) it } + +/** + * Observe a [Promise] as a flow + * + * Warning: Promise will not be canceled on unsubscribe. + */ +fun Promise.asFlow() = callbackFlow { + success { + if (isActive) trySend(it) + close() + } fail { + close(it) + } + + awaitClose() +}