From 2ebd6ebf645fb464e2f26822b4505702b4d74fa4 Mon Sep 17 00:00:00 2001 From: 0x330a <92654767+0x330a@users.noreply.github.com> Date: Thu, 13 Apr 2023 01:31:25 +1000 Subject: [PATCH] fix: fix up the duplicate thread creation in the message receive handler --- .../conversation/v2/ConversationActivityV2.kt | 101 ++++++++---------- .../securesms/database/Storage.kt | 23 ++-- .../res/layout/activity_conversation_v2.xml | 13 +++ app/src/main/res/values/strings.xml | 5 + libsession-util/libsession-util | 2 +- .../libsession/database/StorageProtocol.kt | 2 +- .../messaging/jobs/BatchMessageReceiveJob.kt | 25 ++++- .../libsession/messaging/messages/Message.kt | 1 + .../sending_receiving/MessageReceiver.kt | 3 + .../sending_receiving/MessageSender.kt | 2 +- .../ReceivedMessageHandler.kt | 6 +- 11 files changed, 108 insertions(+), 75 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt index 3ecb1c11ce..8b9d7340c2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt @@ -3,37 +3,30 @@ package org.thoughtcrime.securesms.conversation.v2 import android.Manifest import android.animation.FloatEvaluator import android.animation.ValueAnimator -import android.content.ClipData -import android.content.ClipboardManager -import android.content.Context -import android.content.DialogInterface -import android.content.Intent +import android.content.* import android.content.res.Resources import android.database.Cursor import android.graphics.Rect import android.graphics.Typeface import android.net.Uri -import android.os.AsyncTask -import android.os.Build -import android.os.Bundle -import android.os.Handler -import android.os.Looper +import android.os.* import android.provider.MediaStore +import android.text.SpannableString +import android.text.SpannableStringBuilder +import android.text.SpannedString import android.text.TextUtils +import android.text.style.StyleSpan import android.util.Pair import android.util.TypedValue -import android.view.ActionMode -import android.view.Menu -import android.view.MenuItem -import android.view.MotionEvent -import android.view.View -import android.view.WindowManager +import android.view.* import android.widget.LinearLayout import android.widget.RelativeLayout import android.widget.Toast import androidx.activity.viewModels import androidx.annotation.DimenRes import androidx.appcompat.app.AlertDialog +import androidx.core.text.set +import androidx.core.text.toSpannable import androidx.core.view.drawToBitmap import androidx.core.view.isVisible import androidx.lifecycle.Lifecycle @@ -79,12 +72,8 @@ import org.session.libsession.messaging.sending_receiving.link_preview.LinkPrevi import org.session.libsession.messaging.sending_receiving.quotes.QuoteModel import org.session.libsession.messaging.utilities.SessionId import org.session.libsession.snode.SnodeAPI -import org.session.libsession.utilities.Address +import org.session.libsession.utilities.* import org.session.libsession.utilities.Address.Companion.fromSerialized -import org.session.libsession.utilities.GroupUtil -import org.session.libsession.utilities.MediaTypes -import org.session.libsession.utilities.Stub -import org.session.libsession.utilities.TextSecurePreferences import org.session.libsession.utilities.concurrent.SimpleTask import org.session.libsession.utilities.recipients.Recipient import org.session.libsession.utilities.recipients.RecipientModifiedListener @@ -117,25 +106,10 @@ import org.thoughtcrime.securesms.conversation.v2.messages.VisibleMessageView import org.thoughtcrime.securesms.conversation.v2.messages.VisibleMessageViewDelegate import org.thoughtcrime.securesms.conversation.v2.search.SearchBottomBar import org.thoughtcrime.securesms.conversation.v2.search.SearchViewModel -import org.thoughtcrime.securesms.conversation.v2.utilities.AttachmentManager -import org.thoughtcrime.securesms.conversation.v2.utilities.BaseDialog -import org.thoughtcrime.securesms.conversation.v2.utilities.MentionManagerUtilities -import org.thoughtcrime.securesms.conversation.v2.utilities.MentionUtilities -import org.thoughtcrime.securesms.conversation.v2.utilities.ResendMessageUtilities +import org.thoughtcrime.securesms.conversation.v2.utilities.* import org.thoughtcrime.securesms.crypto.IdentityKeyUtil import org.thoughtcrime.securesms.crypto.MnemonicUtilities -import org.thoughtcrime.securesms.database.GroupDatabase -import org.thoughtcrime.securesms.database.LokiAPIDatabase -import org.thoughtcrime.securesms.database.LokiMessageDatabase -import org.thoughtcrime.securesms.database.LokiThreadDatabase -import org.thoughtcrime.securesms.database.MmsDatabase -import org.thoughtcrime.securesms.database.MmsSmsDatabase -import org.thoughtcrime.securesms.database.ReactionDatabase -import org.thoughtcrime.securesms.database.RecipientDatabase -import org.thoughtcrime.securesms.database.SessionContactDatabase -import org.thoughtcrime.securesms.database.SmsDatabase -import org.thoughtcrime.securesms.database.Storage -import org.thoughtcrime.securesms.database.ThreadDatabase +import org.thoughtcrime.securesms.database.* import org.thoughtcrime.securesms.database.model.MessageId import org.thoughtcrime.securesms.database.model.MessageRecord import org.thoughtcrime.securesms.database.model.MmsMessageRecord @@ -148,26 +122,13 @@ import org.thoughtcrime.securesms.linkpreview.LinkPreviewViewModel import org.thoughtcrime.securesms.linkpreview.LinkPreviewViewModel.LinkPreviewState import org.thoughtcrime.securesms.mediasend.Media import org.thoughtcrime.securesms.mediasend.MediaSendActivity -import org.thoughtcrime.securesms.mms.AudioSlide -import org.thoughtcrime.securesms.mms.GifSlide -import org.thoughtcrime.securesms.mms.GlideApp -import org.thoughtcrime.securesms.mms.ImageSlide -import org.thoughtcrime.securesms.mms.MediaConstraints -import org.thoughtcrime.securesms.mms.Slide -import org.thoughtcrime.securesms.mms.SlideDeck -import org.thoughtcrime.securesms.mms.VideoSlide +import org.thoughtcrime.securesms.mms.* import org.thoughtcrime.securesms.permissions.Permissions import org.thoughtcrime.securesms.reactions.ReactionsDialogFragment import org.thoughtcrime.securesms.reactions.any.ReactWithAnyEmojiDialogFragment -import org.thoughtcrime.securesms.util.ActivityDispatcher -import org.thoughtcrime.securesms.util.ConfigurationMessageUtilities -import org.thoughtcrime.securesms.util.DateUtils -import org.thoughtcrime.securesms.util.MediaUtil -import org.thoughtcrime.securesms.util.SaveAttachmentTask -import org.thoughtcrime.securesms.util.push -import org.thoughtcrime.securesms.util.toPx +import org.thoughtcrime.securesms.util.* import java.lang.ref.WeakReference -import java.util.Locale +import java.util.* import java.util.concurrent.ExecutionException import java.util.concurrent.atomic.AtomicLong import java.util.concurrent.atomic.AtomicReference @@ -401,6 +362,7 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe updateUnreadCountIndicator() updateSubtitle() + updatePlaceholder() setUpBlockedBanner() binding!!.searchBottomBar.setEventListener(this) showOrHideInputIfNeeded() @@ -984,6 +946,37 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe updateUnreadCountIndicator() } + private fun updatePlaceholder() { + val recipient = viewModel.recipient + ?: return Log.w("Loki", "recipient was null in placeholder update") + val binding = binding ?: return + val openGroup = viewModel.openGroup + val (textResource, insertParam) = when { + recipient.isLocalNumber -> R.string.activity_conversation_empty_state_note_to_self to null + openGroup != null && !openGroup.canWrite -> R.string.activity_conversation_empty_state_read_only to recipient.toShortString() + else -> R.string.activity_conversation_empty_state_default to recipient.toShortString() + } + val showPlaceholder = adapter.itemCount == 0 + binding.placeholderText.isVisible = showPlaceholder + if (showPlaceholder) { + if (insertParam != null) { + val span = getText(textResource) as SpannedString + val annotations = span.getSpans(0, span.length, StyleSpan::class.java) + val boldSpan = annotations.first() + val spannedParam = insertParam.toSpannable() + spannedParam[0 until spannedParam.length] = StyleSpan(boldSpan.style) + val originalStart = span.getSpanStart(boldSpan) + val originalEnd = span.getSpanEnd(boldSpan) + val newString = SpannableStringBuilder(span) + .replace(originalStart, originalEnd, spannedParam) + binding.placeholderText.text = newString + } else { + binding.placeholderText.setText(textResource) + } + + } + } + private fun showOrHideScrollToBottomButton(show: Boolean = true) { binding?.scrollToBottomButton?.isVisible = show && !isScrolledToBottom && adapter.itemCount > 0 } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/Storage.kt b/app/src/main/java/org/thoughtcrime/securesms/database/Storage.kt index b916f4474c..0a8aae2d59 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/Storage.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/Storage.kt @@ -98,7 +98,8 @@ class Storage(context: Context, helper: SQLCipherOpenHelper, private val configF // TODO: maybe add time here from formation / creation message override fun threadCreated(address: Address, threadId: Long) { - Log.d("Loki-DBG", "creating thread for $address") + Log.d("Loki-DBG", "creating thread for $address\nExecution context:\n${Thread.currentThread().stackTrace.joinToString("\n")}") + val volatile = configFactory.convoVolatile ?: return if (address.isGroup) { val groups = configFactory.userGroups ?: return @@ -136,7 +137,7 @@ class Storage(context: Context, helper: SQLCipherOpenHelper, private val configF } override fun threadDeleted(address: Address, threadId: Long) { - Log.d("Loki-DBG", "deleting thread for $address") + Log.d("Loki-DBG", "deleting thread for $address\nExecution context:\n${Thread.currentThread().stackTrace.joinToString("\n")}") val volatile = configFactory.convoVolatile ?: return if (address.isGroup) { val groups = configFactory.userGroups ?: return @@ -460,11 +461,11 @@ class Storage(context: Context, helper: SQLCipherOpenHelper, private val configF val extracted = convos.all() for (conversation in extracted) { val threadId = when (conversation) { - is Conversation.OneToOne -> getOrCreateThreadIdFor(fromSerialized(conversation.sessionId)) - is Conversation.LegacyGroup -> getOrCreateThreadIdFor("", conversation.groupId,null) - is Conversation.Community -> getOrCreateThreadIdFor("",null, "${conversation.baseCommunityInfo.baseUrl}.${conversation.baseCommunityInfo.room}") + is Conversation.OneToOne -> getThreadIdFor(conversation.sessionId, null, null, createThread = false) + is Conversation.LegacyGroup -> getThreadIdFor("", conversation.groupId,null, createThread = false) + is Conversation.Community -> getThreadIdFor("",null, "${conversation.baseCommunityInfo.baseUrl}.${conversation.baseCommunityInfo.room}", createThread = false) } - if (threadId >= 0) { + if (threadId != null) { markConversationAsRead(threadId, conversation.lastRead) updateThread(threadId, false) } @@ -981,17 +982,19 @@ class Storage(context: Context, helper: SQLCipherOpenHelper, private val configF return DatabaseComponent.get(context).threadDatabase().getOrCreateThreadIdFor(recipient) } - override fun getOrCreateThreadIdFor(publicKey: String, groupPublicKey: String?, openGroupID: String?): Long { + override fun getThreadIdFor(publicKey: String, groupPublicKey: String?, openGroupID: String?, createThread: Boolean): Long? { val database = DatabaseComponent.get(context).threadDatabase() return if (!openGroupID.isNullOrEmpty()) { val recipient = Recipient.from(context, fromSerialized(GroupUtil.getEncodedOpenGroupID(openGroupID.toByteArray())), false) - database.getThreadIdIfExistsFor(recipient) + database.getThreadIdIfExistsFor(recipient).let { if (it == -1L) null else it } } else if (!groupPublicKey.isNullOrEmpty()) { val recipient = Recipient.from(context, fromSerialized(GroupUtil.doubleEncodeGroupID(groupPublicKey)), false) - database.getOrCreateThreadIdFor(recipient) + if (createThread) database.getOrCreateThreadIdFor(recipient) + else database.getThreadIdIfExistsFor(recipient).let { if (it == -1L) null else it } } else { val recipient = Recipient.from(context, fromSerialized(publicKey), false) - database.getOrCreateThreadIdFor(recipient) + if (createThread) database.getOrCreateThreadIdFor(recipient) + else database.getThreadIdIfExistsFor(recipient).let { if (it == -1L) null else it } } } diff --git a/app/src/main/res/layout/activity_conversation_v2.xml b/app/src/main/res/layout/activity_conversation_v2.xml index 4ec4666d7f..b1079ca892 100644 --- a/app/src/main/res/layout/activity_conversation_v2.xml +++ b/app/src/main/res/layout/activity_conversation_v2.xml @@ -202,6 +202,19 @@ + + Search GIFs? Session will connect to Giphy to provide search results. You will not have full metadata protection when sending GIFs. Some of your devices are using outdated versions. Syncing may be unreliable until they are updated. + + There are no messages in %s. + You have no messages in Note to Self. + You have no messages from %s.\nSend a message to start the conversation! + diff --git a/libsession-util/libsession-util b/libsession-util/libsession-util index 3817b543f5..057318136c 160000 --- a/libsession-util/libsession-util +++ b/libsession-util/libsession-util @@ -1 +1 @@ -Subproject commit 3817b543f5f862b58afd50c285fbccc26bc82ee0 +Subproject commit 057318136c69f582b7fd78a39ee26686d1d5a3ec diff --git a/libsession/src/main/java/org/session/libsession/database/StorageProtocol.kt b/libsession/src/main/java/org/session/libsession/database/StorageProtocol.kt index 67a7adcd00..dbdbb75a46 100644 --- a/libsession/src/main/java/org/session/libsession/database/StorageProtocol.kt +++ b/libsession/src/main/java/org/session/libsession/database/StorageProtocol.kt @@ -155,7 +155,7 @@ interface StorageProtocol { // Thread fun getOrCreateThreadIdFor(address: Address): Long - fun getOrCreateThreadIdFor(publicKey: String, groupPublicKey: String?, openGroupID: String?): Long + fun getThreadIdFor(publicKey: String, groupPublicKey: String?, openGroupID: String?, createThread: Boolean): Long? fun getThreadId(publicKeyOrOpenGroupID: String): Long? fun getThreadId(openGroup: OpenGroup): Long? fun getThreadId(address: Address): Long? diff --git a/libsession/src/main/java/org/session/libsession/messaging/jobs/BatchMessageReceiveJob.kt b/libsession/src/main/java/org/session/libsession/messaging/jobs/BatchMessageReceiveJob.kt index ec70a940b9..530cacc832 100644 --- a/libsession/src/main/java/org/session/libsession/messaging/jobs/BatchMessageReceiveJob.kt +++ b/libsession/src/main/java/org/session/libsession/messaging/jobs/BatchMessageReceiveJob.kt @@ -10,6 +10,7 @@ import nl.komponents.kovenant.task import org.session.libsession.database.StorageProtocol import org.session.libsession.messaging.MessagingModuleConfiguration import org.session.libsession.messaging.messages.Message +import org.session.libsession.messaging.messages.control.ClosedGroupControlMessage import org.session.libsession.messaging.messages.control.ExpirationTimerUpdate import org.session.libsession.messaging.messages.control.UnsendRequest import org.session.libsession.messaging.messages.visible.ParsedMessage @@ -61,13 +62,29 @@ class BatchMessageReceiveJob( private val OPEN_GROUP_ID_KEY = "open_group_id" } - private fun getThreadId(message: Message, storage: StorageProtocol): Long { + private fun shouldCreateThread(parsedMessage: ParsedMessage): Boolean { + val message = parsedMessage.message + return message is VisibleMessage + || !message.isSenderSelf + // TODO: sort out which messages should create threads: message requests? group creation threads? visible messages? others? calls? + +// || (message is ClosedGroupControlMessage && message.kind is ClosedGroupControlMessage.Kind.New) // this was creating threads for self send messages (i.e. synced group creation) +// if (message is ClosedGroupControlMessage && message.kind is ClosedGroupControlMessage.Kind.New) { +// return true +// } +// if (parsedMessage.message.isSenderSelf ) { +// // all the cases where we should add a self send creating the thread, i.e. group invite? visible message? +// } +// !parsedMessage.message.isSenderSelf || parsedMessage.message is VisibleMessage + } + + private fun getThreadId(message: Message, storage: StorageProtocol, shouldCreateThread: Boolean): Long? { val senderOrSync = when (message) { is VisibleMessage -> message.syncTarget ?: message.sender!! is ExpirationTimerUpdate -> message.syncTarget ?: message.sender!! else -> message.sender!! } - return storage.getOrCreateThreadIdFor(senderOrSync, message.groupPublicKey, openGroupID) + return storage.getThreadIdFor(senderOrSync, message.groupPublicKey, openGroupID, createThread = shouldCreateThread) } override suspend fun execute(dispatcherName: String) { @@ -88,9 +105,9 @@ class BatchMessageReceiveJob( try { val (message, proto) = MessageReceiver.parse(data, openGroupMessageServerID, openGroupPublicKey = serverPublicKey) message.serverHash = serverHash - val threadID = getThreadId(message, storage) val parsedParams = ParsedMessage(messageParameters, message, proto) - if (!threadMap.containsKey(threadID)) { + val threadID = getThreadId(message, storage, shouldCreateThread(parsedParams)) + if (threadID != null && !threadMap.containsKey(threadID)) { threadMap[threadID] = mutableListOf(parsedParams) } else { threadMap[threadID]!! += parsedParams diff --git a/libsession/src/main/java/org/session/libsession/messaging/messages/Message.kt b/libsession/src/main/java/org/session/libsession/messaging/messages/Message.kt index d201daa98d..db2a6f5c5a 100644 --- a/libsession/src/main/java/org/session/libsession/messaging/messages/Message.kt +++ b/libsession/src/main/java/org/session/libsession/messaging/messages/Message.kt @@ -11,6 +11,7 @@ abstract class Message { var receivedTimestamp: Long? = null var recipient: String? = null var sender: String? = null + var isSenderSelf: Boolean = false var groupPublicKey: String? = null var openGroupServerMessageID: Long? = null var serverHash: String? = null diff --git a/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/MessageReceiver.kt b/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/MessageReceiver.kt index 7cb5b95b11..108021dcf5 100644 --- a/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/MessageReceiver.kt +++ b/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/MessageReceiver.kt @@ -149,6 +149,9 @@ object MessageReceiver { if (!message.isSelfSendValid && (sender == userPublicKey || isUserBlindedSender)) { throw Error.SelfSend } + if (sender == userPublicKey || isUserBlindedSender) { + message.isSenderSelf = true + } // Guard against control messages in open groups if (isOpenGroupMessage && message !is VisibleMessage) { throw Error.InvalidMessage diff --git a/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/MessageSender.kt b/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/MessageSender.kt index b5cfdd9264..ecaab000bf 100644 --- a/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/MessageSender.kt +++ b/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/MessageSender.kt @@ -440,7 +440,7 @@ object MessageSender { } fun sendNonDurably(message: Message, address: Address): Promise { - val threadID = MessagingModuleConfiguration.shared.storage.getOrCreateThreadIdFor(address) + val threadID = MessagingModuleConfiguration.shared.storage.getThreadId(address) message.threadID = threadID val destination = Destination.from(address) return send(message, destination) diff --git a/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/ReceivedMessageHandler.kt b/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/ReceivedMessageHandler.kt index aa882c6771..a075ff42f1 100644 --- a/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/ReceivedMessageHandler.kt +++ b/libsession/src/main/java/org/session/libsession/messaging/sending_receiving/ReceivedMessageHandler.kt @@ -232,11 +232,9 @@ fun MessageReceiver.handleVisibleMessage( // Get or create thread // FIXME: In case this is an open group this actually * doesn't * create the thread if it doesn't yet // exist. This is intentional, but it's very non-obvious. - val threadID = storage.getOrCreateThreadIdFor(message.syncTarget ?: messageSender!!, message.groupPublicKey, openGroupID) - if (threadID < 0) { + val threadID = storage.getThreadIdFor(message.syncTarget ?: messageSender!!, message.groupPublicKey, openGroupID, createThread = true) // Thread doesn't exist; should only be reached in a case where we are processing open group messages for a no longer existent thread - throw MessageReceiver.Error.NoThread - } + ?: throw MessageReceiver.Error.NoThread val threadRecipient = storage.getRecipientForThread(threadID) val userBlindedKey = openGroupID?.let { val openGroup = storage.getOpenGroup(threadID) ?: return@let null