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.
This commit is contained in:
Mikunj 2019-11-14 10:44:55 +11:00
parent a90b0e70f5
commit 44ccc66ec2
3 changed files with 56 additions and 32 deletions

View File

@ -27,6 +27,7 @@ import org.whispersystems.signalservice.api.SignalServiceMessageSender;
import org.whispersystems.signalservice.api.messages.SignalServiceAttachment; import org.whispersystems.signalservice.api.messages.SignalServiceAttachment;
import org.whispersystems.signalservice.api.messages.SignalServiceAttachmentPointer; import org.whispersystems.signalservice.api.messages.SignalServiceAttachmentPointer;
import org.whispersystems.signalservice.api.push.SignalServiceAddress; import org.whispersystems.signalservice.api.push.SignalServiceAddress;
import org.whispersystems.signalservice.loki.api.LokiStorageAPI;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
@ -84,14 +85,17 @@ public class AttachmentUploadJob extends BaseJob implements InjectableType {
if (databaseAttachment == null) { if (databaseAttachment == null) {
throw new IllegalStateException("Cannot find the specified attachment."); 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(); database.updateAttachmentAfterUpload(databaseAttachment.getAttachmentId(), attachment);
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);
} }
@Override @Override

View File

@ -49,6 +49,8 @@ import org.whispersystems.signalservice.loki.utilities.PromiseUtil;
import java.io.FileNotFoundException; import java.io.FileNotFoundException;
import java.io.IOException; import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedList; import java.util.LinkedList;
import java.util.List; import java.util.List;
@ -94,37 +96,29 @@ public class PushMediaSendJob extends PushSendJob implements InjectableType {
this.shouldSendSyncMessage = shouldSendSyncMessage; this.shouldSendSyncMessage = shouldSendSyncMessage;
} }
@WorkerThread
public static void enqueue(@NonNull Context context, @NonNull JobManager jobManager, long messageId, @NonNull Address destination, boolean shouldSendSyncMessage) { 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) { 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<PushMediaSendJob> jobs) {
if (jobs.size() == 0) { return; }
PushMediaSendJob first = jobs.get(0);
long messageId = first.templateMessageId;
try { try {
MmsDatabase database = DatabaseFactory.getMmsDatabase(context); List<AttachmentUploadJob> attachmentJobs = getAttachmentUploadJobs(context, messageId, first.destination);
OutgoingMediaMessage message = database.getOutgoingMessage(messageId);
List<Attachment> 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<AttachmentUploadJob> attachmentJobs = Stream.of(attachments).map(a -> new AttachmentUploadJob(((DatabaseAttachment) a).getAttachmentId(), destination)).toList();
if (attachmentJobs.isEmpty()) { if (attachmentJobs.isEmpty()) {
jobManager.add(new PushMediaSendJob(templateMessageId, messageId, destination, isFriendRequest, customFriendRequestMessage, shouldSendSyncMessage)); for (PushMediaSendJob job : jobs) { jobManager.add(job); }
} else { } else {
jobManager.startChain(attachmentJobs) jobManager.startChain(attachmentJobs)
.then(new PushMediaSendJob(templateMessageId, messageId, destination, isFriendRequest, customFriendRequestMessage, shouldSendSyncMessage)) .then((List<Job>)(List)jobs)
.enqueue(); .enqueue();
} }
} catch (NoSuchMessageException | MmsException e) { } catch (NoSuchMessageException | MmsException e) {
Log.w(TAG, "Failed to enqueue message.", e); Log.w(TAG, "Failed to enqueue message.", e);
DatabaseFactory.getMmsDatabase(context).markAsSentFailed(messageId); DatabaseFactory.getMmsDatabase(context).markAsSentFailed(messageId);
@ -132,6 +126,20 @@ public class PushMediaSendJob extends PushSendJob implements InjectableType {
} }
} }
public static List<AttachmentUploadJob> getAttachmentUploadJobs(@NonNull Context context, long messageId, @NonNull Address destination)
throws NoSuchMessageException, MmsException
{
MmsDatabase database = DatabaseFactory.getMmsDatabase(context);
OutgoingMediaMessage message = database.getOutgoingMessage(messageId);
List<Attachment> 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 @Override
public @NonNull Data serialize() { public @NonNull Data serialize() {
Data.Builder builder = new Data.Builder() Data.Builder builder = new Data.Builder()

View File

@ -35,6 +35,7 @@ import org.thoughtcrime.securesms.database.SmsDatabase;
import org.thoughtcrime.securesms.database.ThreadDatabase; import org.thoughtcrime.securesms.database.ThreadDatabase;
import org.thoughtcrime.securesms.database.model.MessageRecord; import org.thoughtcrime.securesms.database.model.MessageRecord;
import org.thoughtcrime.securesms.database.model.SmsMessageRecord; import org.thoughtcrime.securesms.database.model.SmsMessageRecord;
import org.thoughtcrime.securesms.jobmanager.Job;
import org.thoughtcrime.securesms.jobmanager.JobManager; import org.thoughtcrime.securesms.jobmanager.JobManager;
import org.thoughtcrime.securesms.jobs.MmsSendJob; import org.thoughtcrime.securesms.jobs.MmsSendJob;
import org.thoughtcrime.securesms.jobs.MultiDeviceContactUpdateJob; 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 org.whispersystems.signalservice.loki.utilities.PromiseUtil;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map; import java.util.Map;
import kotlin.Unit; import kotlin.Unit;
@ -313,6 +316,7 @@ public class MessageSender {
MultiDeviceUtilities.getAllDevicePublicKeysWithFriendStatus(context, recipientPublicKey).success(devices -> { MultiDeviceUtilities.getAllDevicePublicKeysWithFriendStatus(context, recipientPublicKey).success(devices -> {
int friendCount = MultiDeviceUtilities.getFriendCount(context, devices.keySet()); int friendCount = MultiDeviceUtilities.getFriendCount(context, devices.keySet());
Util.runOnMain(() -> { Util.runOnMain(() -> {
ArrayList<Job> jobs = new ArrayList<>();
for (Map.Entry<String, Boolean> entry : devices.entrySet()) { for (Map.Entry<String, Boolean> entry : devices.entrySet()) {
String devicePublicKey = entry.getKey(); String devicePublicKey = entry.getKey();
boolean isFriend = entry.getValue(); 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 // We should also send a sync message if we haven't already sent one
boolean shouldSendSyncMessage = !hasSentSyncMessage[0] && address.isPhone(); boolean shouldSendSyncMessage = !hasSentSyncMessage[0] && address.isPhone();
if (type == MessageType.MEDIA) { if (type == MessageType.MEDIA) {
PushMediaSendJob.enqueue(context, jobManager, messageId, messageIDToUse, address, shouldSendSyncMessage); jobs.add(new PushMediaSendJob(messageId, messageIDToUse, address, false, null, shouldSendSyncMessage));
} else { } else {
jobManager.add(new PushTextSendJob(messageId, messageIDToUse, address, shouldSendSyncMessage)); jobs.add(new PushTextSendJob(messageId, messageIDToUse, address, shouldSendSyncMessage));
} }
if (shouldSendSyncMessage) { hasSentSyncMessage[0] = true; } if (shouldSendSyncMessage) { hasSentSyncMessage[0] = true; }
} else { } else {
@ -336,12 +340,20 @@ public class MessageSender {
boolean isFriendsWithAny = (friendCount > 0); boolean isFriendsWithAny = (friendCount > 0);
String defaultFriendRequestMessage = isFriendsWithAny ? "Accept this friend request to enable messages to be synced across devices" : null; String defaultFriendRequestMessage = isFriendsWithAny ? "Accept this friend request to enable messages to be synced across devices" : null;
if (type == MessageType.MEDIA) { 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 { } 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<PushMediaSendJob>)(List)jobs);
} else {
// Schedule text send jobs
jobManager.startChain(jobs).enqueue();
}
}); });
return Unit.INSTANCE; return Unit.INSTANCE;
}); });