From 44ccc66ec2a9c8b060b85ddab7cb7a8d2a45c213 Mon Sep 17 00:00:00 2001 From: Mikunj Date: Thu, 14 Nov 2019 10:44:55 +1100 Subject: [PATCH] Fix multi-device media messages. This fixes the issue of the same attachments being uploaded multiple times per linked device. Now we only upload the attachments once and then we send the media message. --- .../securesms/jobs/AttachmentUploadJob.java | 18 ++++--- .../securesms/jobs/PushMediaSendJob.java | 50 +++++++++++-------- .../securesms/sms/MessageSender.java | 20 ++++++-- 3 files changed, 56 insertions(+), 32 deletions(-) diff --git a/src/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.java b/src/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.java index 9a091150ce..dce85f83e6 100644 --- a/src/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.java +++ b/src/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.java @@ -27,6 +27,7 @@ import org.whispersystems.signalservice.api.SignalServiceMessageSender; import org.whispersystems.signalservice.api.messages.SignalServiceAttachment; import org.whispersystems.signalservice.api.messages.SignalServiceAttachmentPointer; import org.whispersystems.signalservice.api.push.SignalServiceAddress; +import org.whispersystems.signalservice.loki.api.LokiStorageAPI; import java.io.IOException; import java.io.InputStream; @@ -84,14 +85,17 @@ public class AttachmentUploadJob extends BaseJob implements InjectableType { if (databaseAttachment == null) { throw new IllegalStateException("Cannot find the specified attachment."); } + + // Only upload attachment if necessary + if (databaseAttachment.getUrl().isEmpty()) { + MediaConstraints mediaConstraints = MediaConstraints.getPushMediaConstraints(); + Attachment scaledAttachment = scaleAndStripExif(database, mediaConstraints, databaseAttachment); + SignalServiceAttachment localAttachment = getAttachmentFor(scaledAttachment); + SignalServiceAttachmentPointer remoteAttachment = messageSender.uploadAttachment(localAttachment.asStream(), databaseAttachment.isSticker(), new SignalServiceAddress(destination.serialize())); + Attachment attachment = PointerAttachment.forPointer(Optional.of(remoteAttachment), null, databaseAttachment.getFastPreflightId()).get(); - MediaConstraints mediaConstraints = MediaConstraints.getPushMediaConstraints(); - Attachment scaledAttachment = scaleAndStripExif(database, mediaConstraints, databaseAttachment); - SignalServiceAttachment localAttachment = getAttachmentFor(scaledAttachment); - SignalServiceAttachmentPointer remoteAttachment = messageSender.uploadAttachment(localAttachment.asStream(), databaseAttachment.isSticker(), new SignalServiceAddress(destination.serialize())); - Attachment attachment = PointerAttachment.forPointer(Optional.of(remoteAttachment), null, databaseAttachment.getFastPreflightId()).get(); - - database.updateAttachmentAfterUpload(databaseAttachment.getAttachmentId(), attachment); + database.updateAttachmentAfterUpload(databaseAttachment.getAttachmentId(), attachment); + } } @Override diff --git a/src/org/thoughtcrime/securesms/jobs/PushMediaSendJob.java b/src/org/thoughtcrime/securesms/jobs/PushMediaSendJob.java index 408e353d9c..108bcab936 100644 --- a/src/org/thoughtcrime/securesms/jobs/PushMediaSendJob.java +++ b/src/org/thoughtcrime/securesms/jobs/PushMediaSendJob.java @@ -49,6 +49,8 @@ import org.whispersystems.signalservice.loki.utilities.PromiseUtil; import java.io.FileNotFoundException; import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; import java.util.LinkedList; import java.util.List; @@ -94,37 +96,29 @@ public class PushMediaSendJob extends PushSendJob implements InjectableType { this.shouldSendSyncMessage = shouldSendSyncMessage; } - @WorkerThread public static void enqueue(@NonNull Context context, @NonNull JobManager jobManager, long messageId, @NonNull Address destination, boolean shouldSendSyncMessage) { - enqueue(context, jobManager, messageId, messageId, destination, shouldSendSyncMessage); + enqueue(context, jobManager, messageId, messageId, destination, false, null, shouldSendSyncMessage); } - @WorkerThread - public static void enqueue(@NonNull Context context, @NonNull JobManager jobManager, long templateMessageId, long messageId, @NonNull Address destination, boolean shouldSendSyncMessage) { - enqueue(context, jobManager, templateMessageId, messageId, destination, false, null, shouldSendSyncMessage); - } - - @WorkerThread public static void enqueue(@NonNull Context context, @NonNull JobManager jobManager, long templateMessageId, long messageId, @NonNull Address destination, Boolean isFriendRequest, @Nullable String customFriendRequestMessage, boolean shouldSendSyncMessage) { + enqueue(context, jobManager, Collections.singletonList(new PushMediaSendJob(templateMessageId, messageId, destination, isFriendRequest, customFriendRequestMessage, shouldSendSyncMessage))); + } + + @WorkerThread + public static void enqueue(@NonNull Context context, @NonNull JobManager jobManager, List jobs) { + if (jobs.size() == 0) { return; } + PushMediaSendJob first = jobs.get(0); + long messageId = first.templateMessageId; try { - MmsDatabase database = DatabaseFactory.getMmsDatabase(context); - OutgoingMediaMessage message = database.getOutgoingMessage(messageId); - List attachments = new LinkedList<>(); - - attachments.addAll(message.getAttachments()); - attachments.addAll(Stream.of(message.getLinkPreviews()).filter(p -> p.getThumbnail().isPresent()).map(p -> p.getThumbnail().get()).toList()); - attachments.addAll(Stream.of(message.getSharedContacts()).filter(c -> c.getAvatar() != null).map(c -> c.getAvatar().getAttachment()).withoutNulls().toList()); - - List attachmentJobs = Stream.of(attachments).map(a -> new AttachmentUploadJob(((DatabaseAttachment) a).getAttachmentId(), destination)).toList(); + List attachmentJobs = getAttachmentUploadJobs(context, messageId, first.destination); if (attachmentJobs.isEmpty()) { - jobManager.add(new PushMediaSendJob(templateMessageId, messageId, destination, isFriendRequest, customFriendRequestMessage, shouldSendSyncMessage)); + for (PushMediaSendJob job : jobs) { jobManager.add(job); } } else { jobManager.startChain(attachmentJobs) - .then(new PushMediaSendJob(templateMessageId, messageId, destination, isFriendRequest, customFriendRequestMessage, shouldSendSyncMessage)) - .enqueue(); + .then((List)(List)jobs) + .enqueue(); } - } catch (NoSuchMessageException | MmsException e) { Log.w(TAG, "Failed to enqueue message.", e); DatabaseFactory.getMmsDatabase(context).markAsSentFailed(messageId); @@ -132,6 +126,20 @@ public class PushMediaSendJob extends PushSendJob implements InjectableType { } } + public static List getAttachmentUploadJobs(@NonNull Context context, long messageId, @NonNull Address destination) + throws NoSuchMessageException, MmsException + { + MmsDatabase database = DatabaseFactory.getMmsDatabase(context); + OutgoingMediaMessage message = database.getOutgoingMessage(messageId); + List attachments = new LinkedList<>(); + + attachments.addAll(message.getAttachments()); + attachments.addAll(Stream.of(message.getLinkPreviews()).filter(p -> p.getThumbnail().isPresent()).map(p -> p.getThumbnail().get()).toList()); + attachments.addAll(Stream.of(message.getSharedContacts()).filter(c -> c.getAvatar() != null).map(c -> c.getAvatar().getAttachment()).withoutNulls().toList()); + + return Stream.of(attachments).map(a -> new AttachmentUploadJob(((DatabaseAttachment) a).getAttachmentId(), destination)).toList(); + } + @Override public @NonNull Data serialize() { Data.Builder builder = new Data.Builder() diff --git a/src/org/thoughtcrime/securesms/sms/MessageSender.java b/src/org/thoughtcrime/securesms/sms/MessageSender.java index 996684cd1a..76b8685aaa 100644 --- a/src/org/thoughtcrime/securesms/sms/MessageSender.java +++ b/src/org/thoughtcrime/securesms/sms/MessageSender.java @@ -35,6 +35,7 @@ import org.thoughtcrime.securesms.database.SmsDatabase; import org.thoughtcrime.securesms.database.ThreadDatabase; import org.thoughtcrime.securesms.database.model.MessageRecord; import org.thoughtcrime.securesms.database.model.SmsMessageRecord; +import org.thoughtcrime.securesms.jobmanager.Job; import org.thoughtcrime.securesms.jobmanager.JobManager; import org.thoughtcrime.securesms.jobs.MmsSendJob; import org.thoughtcrime.securesms.jobs.MultiDeviceContactUpdateJob; @@ -69,6 +70,8 @@ import org.whispersystems.signalservice.loki.messaging.LokiThreadFriendRequestSt import org.whispersystems.signalservice.loki.utilities.PromiseUtil; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import kotlin.Unit; @@ -313,6 +316,7 @@ public class MessageSender { MultiDeviceUtilities.getAllDevicePublicKeysWithFriendStatus(context, recipientPublicKey).success(devices -> { int friendCount = MultiDeviceUtilities.getFriendCount(context, devices.keySet()); Util.runOnMain(() -> { + ArrayList jobs = new ArrayList<>(); for (Map.Entry entry : devices.entrySet()) { String devicePublicKey = entry.getKey(); boolean isFriend = entry.getValue(); @@ -325,9 +329,9 @@ public class MessageSender { // We should also send a sync message if we haven't already sent one boolean shouldSendSyncMessage = !hasSentSyncMessage[0] && address.isPhone(); if (type == MessageType.MEDIA) { - PushMediaSendJob.enqueue(context, jobManager, messageId, messageIDToUse, address, shouldSendSyncMessage); + jobs.add(new PushMediaSendJob(messageId, messageIDToUse, address, false, null, shouldSendSyncMessage)); } else { - jobManager.add(new PushTextSendJob(messageId, messageIDToUse, address, shouldSendSyncMessage)); + jobs.add(new PushTextSendJob(messageId, messageIDToUse, address, shouldSendSyncMessage)); } if (shouldSendSyncMessage) { hasSentSyncMessage[0] = true; } } else { @@ -336,12 +340,20 @@ public class MessageSender { boolean isFriendsWithAny = (friendCount > 0); String defaultFriendRequestMessage = isFriendsWithAny ? "Accept this friend request to enable messages to be synced across devices" : null; if (type == MessageType.MEDIA) { - PushMediaSendJob.enqueue(context, jobManager, messageId, messageIDToUse, address, true, defaultFriendRequestMessage, false); + jobs.add(new PushMediaSendJob(messageId, messageIDToUse, address, true, defaultFriendRequestMessage, false)); } else { - jobManager.add(new PushTextSendJob(messageId, messageIDToUse, address, true, defaultFriendRequestMessage, false)); + jobs.add(new PushTextSendJob(messageId, messageIDToUse, address, true, defaultFriendRequestMessage, false)); } } } + + // Start the send + if (type == MessageType.MEDIA) { + PushMediaSendJob.enqueue(context, jobManager, (List)(List)jobs); + } else { + // Schedule text send jobs + jobManager.startChain(jobs).enqueue(); + } }); return Unit.INSTANCE; });