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.

Sign up for GitHub

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 crashes on conversation screen #1590

Merged
merged 2 commits into from
Aug 1, 2024
Merged

Fix crashes on conversation screen #1590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a7e7d0a
Fixed onion request crashes
Aug 1, 2024
db23f89
Use kotlin.Result instead
Aug 1, 2024
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
63 changes: 31 additions & 32 deletions app/src/main/java/org/thoughtcrime/securesms/repository/ConversationRepository.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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))
}
}

Expand Down Expand Up @@ -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))
}
}
}
Expand All @@ -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>()
Expand All @@ -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 {
Expand All @@ -276,67 +275,67 @@ 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()
val openGroup = lokiThreadDb.getOpenGroupChat(threadId)!!

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)
val recipient = reader.current.recipient
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))
}
}

Expand Down
43 changes: 0 additions & 43 deletions app/src/main/java/org/thoughtcrime/securesms/repository/ResultOf.kt
View file
Open in desktop

This file was deleted.

19 changes: 7 additions & 12 deletions app/src/test/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModelTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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() {

Expand Down Expand Up @@ -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)

Expand All @@ -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))

Expand All @@ -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)

Expand All @@ -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)

Expand All @@ -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)

Expand All @@ -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)

Expand All @@ -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
Expand Down