keejef
/
session-android
mirror of https://github.com/oxen-io/session-android
1
0
Fork 0
Code Issues 1 Projects Releases Wiki Activity

Fix crashes on conversation screen (#1590)

Browse Source 
* Fixed onion request crashes

* Use kotlin.Result instead

---------

Co-authored-by: fanchao <git@fanchao.dev>
pull/1594/head
Fanchao Liu 8 months ago committed by GitHub
parent 2a5f86ea06
commit 268b6cc097
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 38 additions and 87 deletions
Show Stats Download Patch File Download Diff File

63
app/src/main/java/org/thoughtcrime/securesms/repository/ConversationRepository.kt
Unescape Escape View File

@ -8,7 +8,6 @@ import app.cash.copper.Query
import app.cash.copper.flow.observeQuery
import dagger.hilt.android.qualifiers.ApplicationContext
import kotlin.coroutines.resume
import kotlin.coroutines.resumeWithException
import kotlin.coroutines.suspendCoroutine
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.map
@ -59,15 +58,15 @@ interface ConversationRepository {
fun deleteLocally(recipient: Recipient, message: MessageRecord)
fun deleteAllLocalMessagesInThreadFromSenderOfMessage(messageRecord: MessageRecord)
fun setApproved(recipient: Recipient, isApproved: Boolean)
suspend fun deleteForEveryone(threadId: Long, recipient: Recipient, message: MessageRecord): ResultOf<Unit>
suspend fun deleteForEveryone(threadId: Long, recipient: Recipient, message: MessageRecord): Result<Unit>
fun buildUnsendRequest(recipient: Recipient, message: MessageRecord): UnsendRequest?
suspend fun deleteMessageWithoutUnsendRequest(threadId: Long, messages: Set<MessageRecord>): ResultOf<Unit>
suspend fun banUser(threadId: Long, recipient: Recipient): ResultOf<Unit>
suspend fun banAndDeleteAll(threadId: Long, recipient: Recipient): ResultOf<Unit>
suspend fun deleteThread(threadId: Long): ResultOf<Unit>
suspend fun deleteMessageRequest(thread: ThreadRecord): ResultOf<Unit>
suspend fun clearAllMessageRequests(block: Boolean): ResultOf<Unit>
suspend fun acceptMessageRequest(threadId: Long, recipient: Recipient): ResultOf<Unit>
suspend fun deleteMessageWithoutUnsendRequest(threadId: Long, messages: Set<MessageRecord>): Result<Unit>
suspend fun banUser(threadId: Long, recipient: Recipient): Result<Unit>
suspend fun banAndDeleteAll(threadId: Long, recipient: Recipient): Result<Unit>
suspend fun deleteThread(threadId: Long): Result<Unit>
suspend fun deleteMessageRequest(thread: ThreadRecord): Result<Unit>
suspend fun clearAllMessageRequests(block: Boolean): Result<Unit>
suspend fun acceptMessageRequest(threadId: Long, recipient: Recipient): Result<Unit>
fun declineMessageRequest(threadId: Long)
fun hasReceived(threadId: Long): Boolean
}
@ -185,7 +184,7 @@ class DefaultConversationRepository @Inject constructor(
threadId: Long,
recipient: Recipient,
message: MessageRecord
): ResultOf<Unit> = suspendCoroutine { continuation ->
): Result<Unit> = suspendCoroutine { continuation ->
buildUnsendRequest(recipient, message)?.let { unsendRequest ->
MessageSender.send(unsendRequest, recipient.address)
}
@ -196,10 +195,10 @@ class DefaultConversationRepository @Inject constructor(
OpenGroupApi.deleteMessage(messageServerID, openGroup.room, openGroup.server)
.success {
messageDataProvider.deleteMessage(message.id, !message.isMms)
continuation.resume(ResultOf.Success(Unit))
continuation.resume(Result.success(Unit))
}.fail { error ->
Log.w("TAG", "Call to OpenGroupApi.deleteForEveryone failed - attempting to resume..")
continuation.resumeWithException(error)
continuation.resume(Result.failure(error))
}
}
@ -229,10 +228,10 @@ class DefaultConversationRepository @Inject constructor(
}
SnodeAPI.deleteMessage(publicKey, listOf(serverHash))
.success {
continuation.resume(ResultOf.Success(Unit))
continuation.resume(Result.success(Unit))
}.fail { error ->
Log.w("ConversationRepository", "Call to SnodeAPI.deleteMessage failed - attempting to resume..")
continuation.resumeWithException(error)
continuation.resume(Result.failure(error))
}
}
}
@ -250,7 +249,7 @@ class DefaultConversationRepository @Inject constructor(
override suspend fun deleteMessageWithoutUnsendRequest(
threadId: Long,
messages: Set<MessageRecord>
): ResultOf<Unit> = suspendCoroutine { continuation ->
): Result<Unit> = suspendCoroutine { continuation ->
val openGroup = lokiThreadDb.getOpenGroupChat(threadId)
if (openGroup != null) {
val messageServerIDs = mutableMapOf<Long, MessageRecord>()
@ -264,7 +263,7 @@ class DefaultConversationRepository @Inject constructor(
.success {
messageDataProvider.deleteMessage(message.id, !message.isMms)
}.fail { error ->
continuation.resumeWithException(error)
continuation.resume(Result.failure(error))
}
}
} else {
@ -276,22 +275,22 @@ class DefaultConversationRepository @Inject constructor(
}
}
}
continuation.resume(ResultOf.Success(Unit))
continuation.resume(Result.success(Unit))
}
override suspend fun banUser(threadId: Long, recipient: Recipient): ResultOf<Unit> =
override suspend fun banUser(threadId: Long, recipient: Recipient): Result<Unit> =
suspendCoroutine { continuation ->
val accountID = recipient.address.toString()
val openGroup = lokiThreadDb.getOpenGroupChat(threadId)!!
OpenGroupApi.ban(accountID, openGroup.room, openGroup.server)
.success {
continuation.resume(ResultOf.Success(Unit))
continuation.resume(Result.success(Unit))
}.fail { error ->
continuation.resumeWithException(error)
continuation.resume(Result.failure(error))
}
}
override suspend fun banAndDeleteAll(threadId: Long, recipient: Recipient): ResultOf<Unit> =
override suspend fun banAndDeleteAll(threadId: Long, recipient: Recipient): Result<Unit> =
suspendCoroutine { continuation ->
// Note: This accountId could be the blinded Id
val accountID = recipient.address.toString()
@ -299,25 +298,25 @@ class DefaultConversationRepository @Inject constructor(
OpenGroupApi.banAndDeleteAll(accountID, openGroup.room, openGroup.server)
.success {
continuation.resume(ResultOf.Success(Unit))
continuation.resume(Result.success(Unit))
}.fail { error ->
continuation.resumeWithException(error)
continuation.resume(Result.failure(error))
}
}
override suspend fun deleteThread(threadId: Long): ResultOf<Unit> {
override suspend fun deleteThread(threadId: Long): Result<Unit> {
sessionJobDb.cancelPendingMessageSendJobs(threadId)
storage.deleteConversation(threadId)
return ResultOf.Success(Unit)
return Result.success(Unit)
}
override suspend fun deleteMessageRequest(thread: ThreadRecord): ResultOf<Unit> {
override suspend fun deleteMessageRequest(thread: ThreadRecord): Result<Unit> {
sessionJobDb.cancelPendingMessageSendJobs(thread.threadId)
storage.deleteConversation(thread.threadId)
return ResultOf.Success(Unit)
return Result.success(Unit)
}
override suspend fun clearAllMessageRequests(block: Boolean): ResultOf<Unit> {
override suspend fun clearAllMessageRequests(block: Boolean): Result<Unit> {
threadDb.readerFor(threadDb.unapprovedConversationList).use { reader ->
while (reader.next != null) {
deleteMessageRequest(reader.current)
@ -325,18 +324,18 @@ class DefaultConversationRepository @Inject constructor(
if (block) { setBlocked(recipient, true) }
}
}
return ResultOf.Success(Unit)
return Result.success(Unit)
}
override suspend fun acceptMessageRequest(threadId: Long, recipient: Recipient): ResultOf<Unit> = suspendCoroutine { continuation ->
override suspend fun acceptMessageRequest(threadId: Long, recipient: Recipient): Result<Unit> = suspendCoroutine { continuation ->
storage.setRecipientApproved(recipient, true)
val message = MessageRequestResponse(true)
MessageSender.send(message, Destination.from(recipient.address), isSyncMessage = recipient.isLocalNumber)
.success {
threadDb.setHasSent(threadId, true)
continuation.resume(ResultOf.Success(Unit))
continuation.resume(Result.success(Unit))
}.fail { error ->
continuation.resumeWithException(error)
continuation.resume(Result.failure(error))
}
}

43
app/src/main/java/org/thoughtcrime/securesms/repository/ResultOf.kt
Unescape Escape View File

@ -1,43 +0,0 @@
package org.thoughtcrime.securesms.repository
import kotlinx.coroutines.CancellationException
sealed class ResultOf<out T> {
data class Success<out R>(val value: R) : ResultOf<R>()
data class Failure(val throwable: Throwable) : ResultOf<Nothing>()
inline fun onFailure(block: (throwable: Throwable) -> Unit) = this.also {
if (this is Failure) {
block(throwable)
}
}
inline fun onSuccess(block: (value: T) -> Unit) = this.also {
if (this is Success) {
block(value)
}
}
inline fun <R> flatMap(mapper: (T) -> R): ResultOf<R> = when (this) {
is Success -> wrap { mapper(value) }
is Failure -> Failure(throwable)
}
fun getOrThrow(): T = when (this) {
is Success -> value
is Failure -> throw throwable
}
companion object {
inline fun <T> wrap(block: () -> T): ResultOf<T> =
try {
Success(block())
} catch (e: CancellationException) {
throw e
} catch (e: Exception) {
Failure(e)
}
}
}

19
app/src/test/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModelTest.kt
Unescape Escape View File

@ -3,14 +3,12 @@ package org.thoughtcrime.securesms.conversation.v2
import com.goterl.lazysodium.utils.KeyPair
import kotlinx.coroutines.flow.emptyFlow
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.runBlocking
import org.hamcrest.CoreMatchers.endsWith
import org.hamcrest.CoreMatchers.equalTo
import org.hamcrest.CoreMatchers.notNullValue
import org.hamcrest.CoreMatchers.nullValue
import org.hamcrest.MatcherAssert.assertThat
import org.junit.Before
import org.junit.BeforeClass
import org.junit.Test
import org.mockito.Mockito
import org.mockito.Mockito.anyLong
@ -20,14 +18,11 @@ import org.mockito.kotlin.any
import org.mockito.kotlin.mock
import org.mockito.kotlin.whenever
import org.session.libsession.utilities.recipients.Recipient
import org.session.libsignal.utilities.Log
import org.thoughtcrime.securesms.BaseViewModelTest
import org.thoughtcrime.securesms.NoOpLogger
import org.thoughtcrime.securesms.database.MmsDatabase
import org.thoughtcrime.securesms.database.Storage
import org.thoughtcrime.securesms.database.model.MessageRecord
import org.thoughtcrime.securesms.repository.ConversationRepository
import org.thoughtcrime.securesms.repository.ResultOf
class ConversationViewModelTest: BaseViewModelTest() {
@ -107,7 +102,7 @@ class ConversationViewModelTest: BaseViewModelTest() {
val message = mock<MessageRecord>()
val error = Throwable()
whenever(repository.deleteForEveryone(anyLong(), any(), any()))
.thenReturn(ResultOf.Failure(error))
.thenReturn(Result.failure(error))
viewModel.deleteForEveryone(message)
@ -120,7 +115,7 @@ class ConversationViewModelTest: BaseViewModelTest() {
val message = mock<MessageRecord>()
val error = Throwable()
whenever(repository.deleteMessageWithoutUnsendRequest(anyLong(), anySet()))
.thenReturn(ResultOf.Failure(error))
.thenReturn(Result.failure(error))
viewModel.deleteMessagesWithoutUnsendRequest(setOf(message))
@ -130,7 +125,7 @@ class ConversationViewModelTest: BaseViewModelTest() {
@Test
fun `should emit error message on ban user failure`() = runBlockingTest {
val error = Throwable()
whenever(repository.banUser(anyLong(), any())).thenReturn(ResultOf.Failure(error))
whenever(repository.banUser(anyLong(), any())).thenReturn(Result.failure(error))
viewModel.banUser(recipient)
@ -139,7 +134,7 @@ class ConversationViewModelTest: BaseViewModelTest() {
@Test
fun `should emit a message on ban user success`() = runBlockingTest {
whenever(repository.banUser(anyLong(), any())).thenReturn(ResultOf.Success(Unit))
whenever(repository.banUser(anyLong(), any())).thenReturn(Result.success(Unit))
viewModel.banUser(recipient)
@ -152,7 +147,7 @@ class ConversationViewModelTest: BaseViewModelTest() {
@Test
fun `should emit error message on ban user and delete all failure`() = runBlockingTest {
val error = Throwable()
whenever(repository.banAndDeleteAll(anyLong(), any())).thenReturn(ResultOf.Failure(error))
whenever(repository.banAndDeleteAll(anyLong(), any())).thenReturn(Result.failure(error))
viewModel.banAndDeleteAll(messageRecord)
@ -161,7 +156,7 @@ class ConversationViewModelTest: BaseViewModelTest() {
@Test
fun `should emit a message on ban user and delete all success`() = runBlockingTest {
whenever(repository.banAndDeleteAll(anyLong(), any())).thenReturn(ResultOf.Success(Unit))
whenever(repository.banAndDeleteAll(anyLong(), any())).thenReturn(Result.success(Unit))
viewModel.banAndDeleteAll(messageRecord)
@ -188,7 +183,7 @@ class ConversationViewModelTest: BaseViewModelTest() {
@Test
fun `should remove shown message`() = runBlockingTest {
// Given that a message is generated
whenever(repository.banUser(anyLong(), any())).thenReturn(ResultOf.Success(Unit))
whenever(repository.banUser(anyLong(), any())).thenReturn(Result.success(Unit))
viewModel.banUser(recipient)
assertThat(viewModel.uiState.value.uiMessages.size, equalTo(1))
// When the message is shown

Write Preview
Loading…
Cancel
Save