From 7945b3c9714aef18d34f3e1161a2f0a5e854066b Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Wed, 30 Nov 2022 16:34:59 -0400 Subject: [PATCH] Fix story sync message behaviour between iOS and Android. --- .../securesms/database/StorySendTable.kt | 28 ++++++------------- .../jobs/MultiDeviceStorySendSyncJob.kt | 17 +++++------ .../jobs/PushDistributionListSendJob.java | 6 ++-- .../securesms/jobs/RemoteDeleteSendJob.java | 4 --- .../api/SignalServiceMessageSender.java | 8 ++++-- .../api/messages/SignalServiceContent.java | 6 ++-- 6 files changed, 30 insertions(+), 39 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/StorySendTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/StorySendTable.kt index a9e6f6331..f87bd4768 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/StorySendTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/StorySendTable.kt @@ -230,16 +230,6 @@ class StorySendTable(context: Context, databaseHelper: SignalDatabase) : Databas return getLocalManifest(sentTimestamp) } - /** - * Gets the manifest after a change to the available distribution lists occurs. - */ - fun getSentStorySyncManifestForUpdate(sentTimestamp: Long): SentStorySyncManifest { - val localManifest: SentStorySyncManifest = getLocalManifest(sentTimestamp) - val entries: List = localManifest.entries - - return SentStorySyncManifest(entries) - } - /** * Applies the given manifest to the local database. This method will: * @@ -331,34 +321,34 @@ class StorySendTable(context: Context, databaseHelper: SignalDatabase) : Databas } } - private fun getLocalManifest(sentTimestamp: Long): SentStorySyncManifest { + fun getLocalManifest(sentTimestamp: Long): SentStorySyncManifest { val entries = readableDatabase.rawQuery( // language=sql """ SELECT $RECIPIENT_ID, $ALLOWS_REPLIES, - $DISTRIBUTION_ID + $DISTRIBUTION_ID, + ${MmsTable.REMOTE_DELETED} FROM $TABLE_NAME - WHERE $TABLE_NAME.$SENT_TIMESTAMP = ? AND ( - SELECT ${MmsTable.REMOTE_DELETED} - FROM ${MmsTable.TABLE_NAME} - WHERE ${MmsTable.ID} = $TABLE_NAME.$MESSAGE_ID - ) = 0 + INNER JOIN ${MmsTable.TABLE_NAME} ON ${MmsTable.TABLE_NAME}.${MmsTable.ID} = $TABLE_NAME.$MESSAGE_ID + WHERE $TABLE_NAME.$SENT_TIMESTAMP = ? """.trimIndent(), arrayOf(sentTimestamp) ).use { cursor -> val results: MutableMap = mutableMapOf() while (cursor.moveToNext()) { + val isRemoteDeleted = CursorUtil.requireBoolean(cursor, MmsTable.REMOTE_DELETED) val recipientId = RecipientId.from(CursorUtil.requireLong(cursor, RECIPIENT_ID)) val distributionId = DistributionId.from(CursorUtil.requireString(cursor, DISTRIBUTION_ID)) + val distributionIdList: List = if (isRemoteDeleted) emptyList() else listOf(distributionId) val allowsReplies = CursorUtil.requireBoolean(cursor, ALLOWS_REPLIES) val entry = results[recipientId]?.let { it.copy( allowedToReply = it.allowedToReply or allowsReplies, - distributionLists = it.distributionLists + distributionId + distributionLists = it.distributionLists + distributionIdList ) - } ?: SentStorySyncManifest.Entry(recipientId, canReply(recipientId, sentTimestamp), listOf(distributionId)) + } ?: SentStorySyncManifest.Entry(recipientId, canReply(recipientId, sentTimestamp), distributionIdList) results[recipientId] = entry } diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/MultiDeviceStorySendSyncJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/MultiDeviceStorySendSyncJob.kt index 9da1c1df5..e2719624d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/MultiDeviceStorySendSyncJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/MultiDeviceStorySendSyncJob.kt @@ -17,8 +17,7 @@ import java.util.concurrent.TimeUnit /** * Transmits a sent sync transcript to linked devices containing the story sync manifest for the given sent timestamp. - * The transmitted message is sent as a recipient update, and will only contain affected recipients that still have a - * live story for the given timestamp. + * The transmitted message will contain all current recipients of a given story. */ class MultiDeviceStorySendSyncJob private constructor(parameters: Parameters, private val sentTimestamp: Long, private val deletedMessageId: Long) : BaseJob(parameters) { @@ -55,20 +54,18 @@ class MultiDeviceStorySendSyncJob private constructor(parameters: Parameters, pr override fun getFactoryKey(): String = KEY override fun onRun() { - val updateManifest = SignalDatabase.storySends.getSentStorySyncManifestForUpdate(sentTimestamp) - - if (updateManifest.entries.isEmpty()) { - Log.i(TAG, "No entries in updated manifest. Dropping.") - return - } - - val recipientsSet = updateManifest.toRecipientsSet() + val updateManifest = SignalDatabase.storySends.getLocalManifest(sentTimestamp) + val recipientsSet: Set = updateManifest.toRecipientsSet() val transcriptMessage: SignalServiceSyncMessage = SignalServiceSyncMessage.forSentTranscript(buildSentTranscript(recipientsSet)) val sendMessageResult = ApplicationDependencies.getSignalServiceMessageSender().sendSyncMessage(transcriptMessage, Optional.empty()) + Log.i(TAG, "Sent transcript message with ${recipientsSet.size} recipients") + if (!sendMessageResult.isSuccess) { throw RetryableException() } + + SignalDatabase.mms.deleteRemotelyDeletedStory(deletedMessageId) } override fun onShouldRetry(e: Exception): Boolean { diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushDistributionListSendJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushDistributionListSendJob.java index 405df55ac..9c8061686 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushDistributionListSendJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushDistributionListSendJob.java @@ -138,8 +138,8 @@ public final class PushDistributionListSendJob extends PushSendJob { public void onPushSend() throws IOException, MmsException, NoSuchMessageException, RetryLaterException { - MessageTable database = SignalDatabase.mms(); - OutgoingMediaMessage message = database.getOutgoingMessage(messageId); + MessageTable database = SignalDatabase.mms(); + OutgoingMediaMessage message = database.getOutgoingMessage(messageId); Set existingNetworkFailures = message.getNetworkFailures(); Set existingIdentityMismatches = message.getIdentityKeyMismatches(); @@ -213,6 +213,8 @@ public final class PushDistributionListSendJob extends PushSendJob { SentStorySyncManifest manifest = SignalDatabase.storySends().getFullSentStorySyncManifest(messageId, message.getSentTimeMillis()); Set manifestCollection = manifest != null ? manifest.toRecipientsSet() : Collections.emptySet(); + Log.d(TAG, "[" + messageId + "] Sending a story message with a manifest of size " + manifestCollection.size()); + return GroupSendUtil.sendStoryMessage(context, message.getRecipient().requireDistributionListId(), destinations, isRecipientUpdate, new MessageId(messageId, true), message.getSentTimeMillis(), storyMessage, manifestCollection); } catch (ServerRejectedException e) { throw new UndeliverableMessageException(e); diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/RemoteDeleteSendJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/RemoteDeleteSendJob.java index 3e776acc7..71ab975f6 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/RemoteDeleteSendJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/RemoteDeleteSendJob.java @@ -189,10 +189,6 @@ public class RemoteDeleteSendJob extends BaseJob { if (recipients.isEmpty()) { db.markAsSent(messageId, true); - - if (MessageRecordUtil.isStory(message)) { - db.deleteRemotelyDeletedStory(messageId); - } } else { Log.w(TAG, "Still need to send to " + recipients.size() + " recipients. Retrying."); throw new RetryLaterException(); diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageSender.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageSender.java index 9b04ced56..2148f38e0 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageSender.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageSender.java @@ -278,6 +278,11 @@ public class SignalServiceMessageSender { Set manifest) throws IOException, UntrustedIdentityException { + if (manifest.isEmpty()) { + Log.w(TAG, "Refusing to send sync message for empty manifest."); + return; + } + SignalServiceSyncMessage syncMessage = createSelfSendSyncMessageForStory(message, timestamp, isRecipientUpdate, manifest); sendSyncMessage(syncMessage, Optional.empty()); } @@ -300,8 +305,7 @@ public class SignalServiceMessageSender { List sendMessageResults = sendGroupMessage(distributionId, recipients, unidentifiedAccess, timestamp, content, ContentHint.IMPLICIT, groupId, false, SenderKeyGroupEvents.EMPTY, false, true); if (aciStore.isMultiDevice()) { - SignalServiceSyncMessage syncMessage = createSelfSendSyncMessageForStory(message, timestamp, isRecipientUpdate, manifest); - sendSyncMessage(syncMessage, Optional.empty()); + sendStorySyncMessage(message, timestamp, isRecipientUpdate, manifest); } return sendMessageResults; diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/messages/SignalServiceContent.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/messages/SignalServiceContent.java index 83597650d..6cdda8678 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/messages/SignalServiceContent.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/messages/SignalServiceContent.java @@ -763,8 +763,10 @@ public final class SignalServiceContent { if (!address.isPresent() && !dataMessage.flatMap(SignalServiceDataMessage::getGroupContext).isPresent() && - !storyMessage.flatMap(SignalServiceStoryMessage::getGroupContext).isPresent()) { - throw new InvalidMessageStructureException("SyncMessage missing both destination and group ID!"); + !storyMessage.flatMap(SignalServiceStoryMessage::getGroupContext).isPresent() && + recipientManifest.isEmpty()) + { + throw new InvalidMessageStructureException("SyncMessage missing destination, group ID, and recipient manifest!"); } for (SignalServiceProtos.SyncMessage.Sent.UnidentifiedDeliveryStatus status : sentContent.getUnidentifiedStatusList()) {