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 d0a0298e1c..00e72562d5 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 @@ -46,7 +46,6 @@ import androidx.lifecycle.Observer import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.flowWithLifecycle import androidx.lifecycle.lifecycleScope -import androidx.lifecycle.repeatOnLifecycle import androidx.loader.app.LoaderManager import androidx.loader.content.Loader import androidx.recyclerview.widget.LinearLayoutManager @@ -57,8 +56,6 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.channels.BufferOverflow import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.collectLatest -import kotlinx.coroutines.flow.consumeAsFlow -import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.launch import kotlinx.coroutines.withContext @@ -181,8 +178,6 @@ import org.thoughtcrime.securesms.util.push import org.thoughtcrime.securesms.util.show import org.thoughtcrime.securesms.util.toPx import java.lang.ref.WeakReference -import java.time.Instant -import java.util.Date import java.util.Locale import java.util.concurrent.ExecutionException import java.util.concurrent.atomic.AtomicBoolean @@ -193,8 +188,6 @@ import kotlin.math.abs import kotlin.math.min import kotlin.math.roundToInt import kotlin.math.sqrt -import kotlin.time.Duration -import kotlin.time.Duration.Companion.milliseconds private const val TAG = "ConversationActivityV2" @@ -1896,7 +1889,6 @@ class ConversationActivityV2 : PassphraseRequiredActionBarActivity(), InputBarDe // If the recipient is a community then we delete the message for everyone if (recipient.isCommunityRecipient) { val messageCount = 1 // Only used for plurals string - showSessionDialog { title(resources.getQuantityString(R.plurals.ConversationFragment_delete_selected_messages, messageCount, messageCount)) text(resources.getQuantityString(R.plurals.ConversationFragment_this_will_permanently_delete_all_n_selected_messages, messageCount, messageCount)) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationReactionOverlay.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationReactionOverlay.kt index d48d761f94..d336c5fcce 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationReactionOverlay.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationReactionOverlay.kt @@ -39,7 +39,6 @@ import org.thoughtcrime.securesms.components.menu.ActionItem import org.thoughtcrime.securesms.conversation.v2.menus.ConversationMenuItemHelper.userCanBanSelectedUsers import org.thoughtcrime.securesms.conversation.v2.menus.ConversationMenuItemHelper.userCanDeleteSelectedItems import org.thoughtcrime.securesms.database.MmsSmsDatabase -import org.thoughtcrime.securesms.database.SessionContactDatabase import org.thoughtcrime.securesms.database.model.MediaMmsMessageRecord import org.thoughtcrime.securesms.database.model.MessageRecord import org.thoughtcrime.securesms.database.model.ReactionRecord diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/MmsDatabase.kt b/app/src/main/java/org/thoughtcrime/securesms/database/MmsDatabase.kt index 15a515ee5b..0f39e3a496 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MmsDatabase.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MmsDatabase.kt @@ -630,6 +630,7 @@ class MmsDatabase(context: Context, databaseHelper: SQLCipherOpenHelper) : Messa if (retrieved.isExpirationUpdate) deleteExpirationTimerMessages(threadId, true.takeUnless { retrieved.isGroup }) val messageId = insertMessageOutbox(retrieved, threadId, false, null, serverTimestamp, runThreadUpdate) if (messageId == -1L) { + Log.w(TAG, "insertSecureDecryptedMessageOutbox believes the MmsDatabase insertion failed.") return Optional.absent() } markAsSent(messageId, true) diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsDatabase.java index 98f118d59d..1bf1dcdb15 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MmsSmsDatabase.java @@ -30,6 +30,7 @@ import net.zetetic.database.sqlcipher.SQLiteQueryBuilder; import org.jetbrains.annotations.NotNull; import org.session.libsession.utilities.Address; import org.session.libsession.utilities.Util; +import org.session.libsignal.utilities.Log; import org.thoughtcrime.securesms.database.MessagingDatabase.SyncMessageId; import org.thoughtcrime.securesms.database.helpers.SQLCipherOpenHelper; import org.thoughtcrime.securesms.database.model.MessageRecord; @@ -115,6 +116,53 @@ public class MmsSmsDatabase extends Database { return null; } + public @Nullable MessageRecord getSentMessageFor(long timestamp, String serializedAuthor) { + // Early exit if the author is not us + boolean isOwnNumber = Util.isOwnNumber(context, serializedAuthor); + if (!isOwnNumber) { + Log.i(TAG, "Asked to find sent messages but provided author is not us - returning null."); + return null; + } + + try (Cursor cursor = queryTables(PROJECTION, MmsSmsColumns.NORMALIZED_DATE_SENT + " = " + timestamp, null, null)) { + MmsSmsDatabase.Reader reader = readerFor(cursor); + + MessageRecord messageRecord; + while ((messageRecord = reader.getNext()) != null) { + if (messageRecord.isOutgoing()) + { + return messageRecord; + } + } + } + Log.i(TAG, "Could not find any message sent from us at provided timestamp - returning null."); + return null; + } + + public MessageRecord getLastSentMessageRecordFromSender(long threadId, String serializedAuthor) { + // Early exit if the author is not us + boolean isOwnNumber = Util.isOwnNumber(context, serializedAuthor); + if (!isOwnNumber) { + Log.i(TAG, "Asked to find last sent message but provided author is not us - returning null."); + return null; + } + + String order = MmsSmsColumns.NORMALIZED_DATE_SENT + " DESC"; + String selection = MmsSmsColumns.THREAD_ID + " = " + threadId; + + // Try everything with resources so that they auto-close on end of scope + try (Cursor cursor = queryTables(PROJECTION, selection, order, null)) { + try (MmsSmsDatabase.Reader reader = readerFor(cursor)) { + MessageRecord messageRecord; + while ((messageRecord = reader.getNext()) != null) { + if (messageRecord.isOutgoing()) { return messageRecord; } + } + } + } + Log.i(TAG, "Could not find last sent message from us in given thread - returning null."); + return null; + } + public @Nullable MessageRecord getMessageFor(long timestamp, Address author) { return getMessageFor(timestamp, author.serialize()); } @@ -248,20 +296,27 @@ public class MmsSmsDatabase extends Database { } public long getLastSentMessageFromSender(long threadId, String serializedAuthor) { + + // Early exit + boolean isOwnNumber = Util.isOwnNumber(context, serializedAuthor); + if (!isOwnNumber) { + Log.i(TAG, "Asked to find last sent message but sender isn't us - returning null."); + return -1; + } + String order = MmsSmsColumns.NORMALIZED_DATE_SENT + " DESC"; String selection = MmsSmsColumns.THREAD_ID + " = " + threadId; - boolean isOwnNumber = Util.isOwnNumber(context, serializedAuthor); - // Try everything with resources so that they auto-close on end of scope try (Cursor cursor = queryTables(PROJECTION, selection, order, null)) { try (MmsSmsDatabase.Reader reader = readerFor(cursor)) { MessageRecord messageRecord; while ((messageRecord = reader.getNext()) != null) { - if (isOwnNumber && messageRecord.isOutgoing()) { return messageRecord.id; } + if (messageRecord.isOutgoing()) { return messageRecord.id; } } } } + Log.i(TAG, "Could not find last sent message from us - returning -1."); return -1; } 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 ae0569e42a..73dadda2cb 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/Storage.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/Storage.kt @@ -92,7 +92,6 @@ import org.thoughtcrime.securesms.mms.PartAuthority import org.thoughtcrime.securesms.util.ConfigurationMessageUtilities import org.thoughtcrime.securesms.util.SessionMetaProtocol import java.security.MessageDigest -import kotlin.time.Duration.Companion.days import network.loki.messenger.libsession_util.util.Contact as LibSessionContact private const val TAG = "Storage" @@ -767,13 +766,36 @@ open class Storage( override fun markAsSent(timestamp: Long, author: String) { val database = DatabaseComponent.get(context).mmsSmsDatabase() - val messageRecord = database.getMessageFor(timestamp, author) ?: return + val messageRecord = database.getSentMessageFor(timestamp, author) + if (messageRecord == null) { + Log.w(TAG, "Failed to retrieve local message record in Storage.markAsSent - aborting.") + return + } + if (messageRecord.isMms) { - val mmsDatabase = DatabaseComponent.get(context).mmsDatabase() - mmsDatabase.markAsSent(messageRecord.getId(), true) + DatabaseComponent.get(context).mmsDatabase().markAsSent(messageRecord.getId(), true) } else { - val smsDatabase = DatabaseComponent.get(context).smsDatabase() - smsDatabase.markAsSent(messageRecord.getId(), true) + DatabaseComponent.get(context).smsDatabase().markAsSent(messageRecord.getId(), true) + } + } + + // Method that marks a message as sent in Communities (only!) - where the server modifies the + // message timestamp and as such we cannot use that to identify the local message. + override fun markAsSentToCommunity(threadId: Long, messageID: Long) { + val database = DatabaseComponent.get(context).mmsSmsDatabase() + val message = database.getLastSentMessageRecordFromSender(threadId, TextSecurePreferences.getLocalNumber(context)) + + // Ensure we can find the local message.. + if (message == null) { + Log.w(TAG, "Could not find local message in Storage.markAsSentToCommunity - aborting.") + return + } + + // ..and mark as sent if found. + if (message.isMms) { + DatabaseComponent.get(context).mmsDatabase().markAsSent(message.getId(), true) + } else { + DatabaseComponent.get(context).smsDatabase().markAsSent(message.getId(), true) } } @@ -808,7 +830,11 @@ open class Storage( override fun markUnidentified(timestamp: Long, author: String) { val database = DatabaseComponent.get(context).mmsSmsDatabase() - val messageRecord = database.getMessageFor(timestamp, author) ?: return + val messageRecord = database.getMessageFor(timestamp, author) + if (messageRecord == null) { + Log.w(TAG, "Could not identify message with timestamp: $timestamp from author: $author") + return + } if (messageRecord.isMms) { val mmsDatabase = DatabaseComponent.get(context).mmsDatabase() mmsDatabase.markUnidentified(messageRecord.getId(), true) @@ -818,6 +844,26 @@ open class Storage( } } + // Method that marks a message as unidentified in Communities (only!) - where the server + // modifies the message timestamp and as such we cannot use that to identify the local message. + override fun markUnidentifiedInCommunity(threadId: Long, messageId: Long) { + val database = DatabaseComponent.get(context).mmsSmsDatabase() + val message = database.getLastSentMessageRecordFromSender(threadId, TextSecurePreferences.getLocalNumber(context)) + + // Check to ensure the message exists + if (message == null) { + Log.w(TAG, "Could not find local message in Storage.markUnidentifiedInCommunity - aborting.") + return + } + + // Mark it as unidentified if we found the message successfully + if (message.isMms) { + DatabaseComponent.get(context).mmsDatabase().markUnidentified(message.getId(), true) + } else { + DatabaseComponent.get(context).smsDatabase().markUnidentified(message.getId(), true) + } + } + override fun markAsSentFailed(timestamp: Long, author: String, error: Exception) { val database = DatabaseComponent.get(context).mmsSmsDatabase() val messageRecord = database.getMessageFor(timestamp, author) ?: return @@ -971,7 +1017,10 @@ open class Storage( val infoMessage = OutgoingGroupMediaMessage(recipient, updateData, groupID, null, sentTimestamp, 0, 0, true, null, listOf(), listOf()) val mmsDB = DatabaseComponent.get(context).mmsDatabase() val mmsSmsDB = DatabaseComponent.get(context).mmsSmsDatabase() - if (mmsSmsDB.getMessageFor(sentTimestamp, userPublicKey) != null) return + if (mmsSmsDB.getMessageFor(sentTimestamp, userPublicKey) != null) { + Log.w(TAG, "Bailing from insertOutgoingInfoMessage because we believe the message has already been sent!") + return + } val infoMessageID = mmsDB.insertMessageOutbox(infoMessage, threadID, false, null, runThreadUpdate = true) mmsDB.markAsSent(infoMessageID, true) } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/model/DisplayRecord.java b/app/src/main/java/org/thoughtcrime/securesms/database/model/DisplayRecord.java index 39fba182aa..605118f3cc 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/model/DisplayRecord.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/model/DisplayRecord.java @@ -22,7 +22,10 @@ import android.text.SpannableString; import androidx.annotation.NonNull; import org.session.libsession.utilities.recipients.Recipient; +import org.session.libsignal.utilities.Log; +import org.thoughtcrime.securesms.database.MmsDatabase; import org.thoughtcrime.securesms.database.MmsSmsColumns; +import org.thoughtcrime.securesms.database.MmsSmsDatabase; import org.thoughtcrime.securesms.database.SmsDatabase; /** @@ -48,6 +51,9 @@ public abstract class DisplayRecord { long dateReceived, long threadId, int deliveryStatus, int deliveryReceiptCount, long type, int readReceiptCount) { + // TODO: This gets hit very, very often and it likely shouldn't - place a Log.d statement in it to see. + //Log.d("[ACL]", "Creating a display record with delivery status of: " + deliveryStatus); + this.threadId = threadId; this.recipient = recipient; this.dateSent = dateSent; @@ -76,9 +82,7 @@ public abstract class DisplayRecord { && deliveryStatus < SmsDatabase.Status.STATUS_PENDING) || deliveryReceiptCount > 0; } - public boolean isSent() { - return !isFailed() && !isPending(); - } + public boolean isSent() { return MmsSmsColumns.Types.isSentType(type); } public boolean isSyncing() { return MmsSmsColumns.Types.isSyncingType(type); @@ -99,9 +103,10 @@ public abstract class DisplayRecord { } public boolean isPending() { - return MmsSmsColumns.Types.isPendingMessageType(type) - && !MmsSmsColumns.Types.isIdentityVerified(type) - && !MmsSmsColumns.Types.isIdentityDefault(type); + boolean isPending = MmsSmsColumns.Types.isPendingMessageType(type) && + !MmsSmsColumns.Types.isIdentityVerified(type) && + !MmsSmsColumns.Types.isIdentityDefault(type); + return isPending; } public boolean isRead() { return readReceiptCount > 0; } 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 9d6ae18d0c..260e254fe7 100644 --- a/libsession/src/main/java/org/session/libsession/database/StorageProtocol.kt +++ b/libsession/src/main/java/org/session/libsession/database/StorageProtocol.kt @@ -120,7 +120,9 @@ interface StorageProtocol { fun markAsSyncing(timestamp: Long, author: String) fun markAsSending(timestamp: Long, author: String) fun markAsSent(timestamp: Long, author: String) + fun markAsSentToCommunity(threadID: Long, messageID: Long) fun markUnidentified(timestamp: Long, author: String) + fun markUnidentifiedInCommunity(threadID: Long, messageID: Long) fun markAsSyncFailed(timestamp: Long, author: String, error: Exception) fun markAsSentFailed(timestamp: Long, author: String, error: Exception) fun clearErrorMessage(messageID: Long) 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 21df7fcb5d..b0459de1d6 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 @@ -39,6 +39,7 @@ import org.session.libsignal.crypto.PushTransportDetails import org.session.libsignal.protos.SignalServiceProtos import org.session.libsignal.utilities.Base64 import org.session.libsignal.utilities.IdPrefix +import org.session.libsignal.utilities.Log import org.session.libsignal.utilities.Namespace import org.session.libsignal.utilities.defaultRequiresAuth import org.session.libsignal.utilities.hasNamespaces @@ -370,7 +371,7 @@ object MessageSender { } // Result Handling - fun handleSuccessfulMessageSend(message: Message, destination: Destination, isSyncMessage: Boolean = false, openGroupSentTimestamp: Long = -1) { + private fun handleSuccessfulMessageSend(message: Message, destination: Destination, isSyncMessage: Boolean = false, openGroupSentTimestamp: Long = -1) { val storage = MessagingModuleConfiguration.shared.storage val userPublicKey = storage.getUserPublicKey()!! val timestamp = message.sentTimestamp!! @@ -392,8 +393,10 @@ object MessageSender { // in case any errors from previous sends storage.clearErrorMessage(messageID) + // Track the open group server message ID - if (message.openGroupServerMessageID != null && (destination is Destination.LegacyOpenGroup || destination is Destination.OpenGroup)) { + val messageIsAddressedToCommunity = message.openGroupServerMessageID != null && (destination is Destination.LegacyOpenGroup || destination is Destination.OpenGroup) + if (messageIsAddressedToCommunity) { val server: String val room: String when (destination) { @@ -415,9 +418,26 @@ object MessageSender { storage.setOpenGroupServerMessageID(messageID, message.openGroupServerMessageID!!, threadID, !(message as VisibleMessage).isMediaMessage()) } } - // Mark the message as sent - storage.markAsSent(timestamp, userPublicKey) - storage.markUnidentified(timestamp, userPublicKey) + + // Mark the message as sent. + // Note: When sending a message to a community the server modifies the message timestamp + // so when we go to look up the message in the local database by timestamp it fails and + // we're left with the message delivery status as "Sending" forever! As such, we use a + // pair of modified "markAsSentToCommunity" and "markUnidentifiedInCommunity" methods + // to retrieve the local message by thread & message ID rather than timestamp when + // handling community messages only so we can tick the delivery status over to 'Sent'. + // Fixed in: https://optf.atlassian.net/browse/SES-1567 + if (messageIsAddressedToCommunity) + { + storage.markAsSentToCommunity(message.threadID!!, message.id!!) + storage.markUnidentifiedInCommunity(message.threadID!!, message.id!!) + } + else + { + storage.markAsSent(timestamp, userPublicKey) + storage.markUnidentified(timestamp, userPublicKey) + } + // Start the disappearing messages timer if needed SSKEnvironment.shared.messageExpirationManager.maybeStartExpiration(message, startDisappearAfterRead = true) } ?: run {