From 56a44ae65cffe43b15cd4ece3ffabb5a7f98ba33 Mon Sep 17 00:00:00 2001 From: Cody Henthorne Date: Thu, 2 Feb 2023 16:29:42 -0500 Subject: [PATCH] Enforce expected ordering when scheduling text and media messages. --- .../securesms/jobs/IndividualSendJob.java | 12 ++++++------ .../securesms/jobs/PushGroupSendJob.java | 9 +++++---- .../securesms/recipients/RecipientId.java | 4 ++++ .../securesms/service/ScheduledMessageManager.kt | 4 ++-- .../thoughtcrime/securesms/sms/MessageSender.java | 12 ++++++------ 5 files changed, 23 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/IndividualSendJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/IndividualSendJob.java index 96a02ebae0..7d1875d945 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/IndividualSendJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/IndividualSendJob.java @@ -70,9 +70,9 @@ public class IndividualSendJob extends PushSendJob { private final long messageId; - public IndividualSendJob(long messageId, @NonNull Recipient recipient, boolean hasMedia) { + public IndividualSendJob(long messageId, @NonNull Recipient recipient, boolean hasMedia, boolean isScheduledSend) { this(new Parameters.Builder() - .setQueue(recipient.getId().toQueueKey(hasMedia)) + .setQueue(isScheduledSend ? recipient.getId().toScheduledSendQueueKey() : recipient.getId().toQueueKey(hasMedia)) .addConstraint(NetworkConstraint.KEY) .setLifespan(TimeUnit.DAYS.toMillis(1)) .setMaxAttempts(Parameters.UNLIMITED) @@ -85,7 +85,7 @@ public class IndividualSendJob extends PushSendJob { this.messageId = messageId; } - public static Job create(long messageId, @NonNull Recipient recipient, boolean hasMedia) { + public static Job create(long messageId, @NonNull Recipient recipient, boolean hasMedia, boolean isScheduledSend) { if (!recipient.hasServiceId()) { throw new AssertionError("No ServiceId!"); } @@ -94,11 +94,11 @@ public class IndividualSendJob extends PushSendJob { throw new AssertionError("This job does not send group messages!"); } - return new IndividualSendJob(messageId, recipient, hasMedia); + return new IndividualSendJob(messageId, recipient, hasMedia, isScheduledSend); } @WorkerThread - public static void enqueue(@NonNull Context context, @NonNull JobManager jobManager, long messageId, @NonNull Recipient recipient) { + public static void enqueue(@NonNull Context context, @NonNull JobManager jobManager, long messageId, @NonNull Recipient recipient, boolean isScheduledSend) { try { OutgoingMessage message = SignalDatabase.messages().getOutgoingMessage(messageId); if (message.getScheduledDate() != -1) { @@ -107,7 +107,7 @@ public class IndividualSendJob extends PushSendJob { } Set attachmentUploadIds = enqueueCompressingAndUploadAttachmentsChains(jobManager, message); - jobManager.add(IndividualSendJob.create(messageId, recipient, attachmentUploadIds.size() > 0), attachmentUploadIds, recipient.getId().toQueueKey()); + jobManager.add(IndividualSendJob.create(messageId, recipient, attachmentUploadIds.size() > 0, isScheduledSend), attachmentUploadIds, isScheduledSend ? null : recipient.getId().toQueueKey()); } catch (NoSuchMessageException | MmsException e) { Log.w(TAG, "Failed to enqueue message.", e); SignalDatabase.messages().markAsSentFailed(messageId); diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSendJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSendJob.java index 49cd4fa737..9bb6751ed2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSendJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSendJob.java @@ -83,9 +83,9 @@ public final class PushGroupSendJob extends PushSendJob { private final long messageId; private final Set filterRecipients; - public PushGroupSendJob(long messageId, @NonNull RecipientId destination, @NonNull Set filterRecipients, boolean hasMedia) { + public PushGroupSendJob(long messageId, @NonNull RecipientId destination, @NonNull Set filterRecipients, boolean hasMedia, boolean isScheduledSend) { this(new Job.Parameters.Builder() - .setQueue(destination.toQueueKey(hasMedia)) + .setQueue(isScheduledSend ? destination.toScheduledSendQueueKey() : destination.toQueueKey(hasMedia)) .addConstraint(NetworkConstraint.KEY) .setLifespan(TimeUnit.DAYS.toMillis(1)) .setMaxAttempts(Parameters.UNLIMITED) @@ -106,7 +106,8 @@ public final class PushGroupSendJob extends PushSendJob { @NonNull JobManager jobManager, long messageId, @NonNull RecipientId destination, - @NonNull Set filterAddresses) + @NonNull Set filterAddresses, + boolean isScheduledSend) { try { Recipient group = Recipient.resolved(destination); @@ -135,7 +136,7 @@ public final class PushGroupSendJob extends PushSendJob { throw new MmsException("Inactive group!"); } - jobManager.add(new PushGroupSendJob(messageId, destination, filterAddresses, !attachmentUploadIds.isEmpty()), attachmentUploadIds, attachmentUploadIds.isEmpty() ? null : destination.toQueueKey()); + jobManager.add(new PushGroupSendJob(messageId, destination, filterAddresses, !attachmentUploadIds.isEmpty(), isScheduledSend), attachmentUploadIds, attachmentUploadIds.isEmpty() || isScheduledSend ? null : destination.toQueueKey()); } catch (NoSuchMessageException | MmsException e) { Log.w(TAG, "Failed to enqueue message.", e); diff --git a/app/src/main/java/org/thoughtcrime/securesms/recipients/RecipientId.java b/app/src/main/java/org/thoughtcrime/securesms/recipients/RecipientId.java index 9227efd698..bc8eb7e8b1 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/recipients/RecipientId.java +++ b/app/src/main/java/org/thoughtcrime/securesms/recipients/RecipientId.java @@ -158,6 +158,10 @@ public class RecipientId implements Parcelable, Comparable, Databas return "RecipientId::" + id + (forMedia ? "::MEDIA" : ""); } + public @NonNull String toScheduledSendQueueKey() { + return "RecipientId::" + id + "::SCHEDULED"; + } + @Override public @NonNull String toString() { return "RecipientId::" + id; diff --git a/app/src/main/java/org/thoughtcrime/securesms/service/ScheduledMessageManager.kt b/app/src/main/java/org/thoughtcrime/securesms/service/ScheduledMessageManager.kt index c5990d9b19..bd630166b0 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/service/ScheduledMessageManager.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/service/ScheduledMessageManager.kt @@ -45,9 +45,9 @@ class ScheduledMessageManager( for (record in scheduledMessagesToSend) { if (SignalDatabase.messages.clearScheduledStatus(record.threadId, record.id)) { if (record.recipient.isPushGroup) { - PushGroupSendJob.enqueue(application, ApplicationDependencies.getJobManager(), record.id, record.recipient.id, emptySet()) + PushGroupSendJob.enqueue(application, ApplicationDependencies.getJobManager(), record.id, record.recipient.id, emptySet(), true) } else { - IndividualSendJob.enqueue(application, ApplicationDependencies.getJobManager(), record.id, record.recipient) + IndividualSendJob.enqueue(application, ApplicationDependencies.getJobManager(), record.id, record.recipient, true) } } else { Log.i(TAG, "messageId=${record.id} was not a scheduled message, ignoring") diff --git a/app/src/main/java/org/thoughtcrime/securesms/sms/MessageSender.java b/app/src/main/java/org/thoughtcrime/securesms/sms/MessageSender.java index 9989e4c63d..e1a42c406d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/sms/MessageSender.java +++ b/app/src/main/java/org/thoughtcrime/securesms/sms/MessageSender.java @@ -394,11 +394,11 @@ public class MessageSender { if (isLocalSelfSend(context, recipient, SendType.SIGNAL)) { sendLocalMediaSelf(context, messageId); } else if (recipient.isPushGroup()) { - jobManager.add(new PushGroupSendJob(messageId, recipient.getId(), Collections.emptySet(), true), messageDependsOnIds, recipient.getId().toQueueKey()); + jobManager.add(new PushGroupSendJob(messageId, recipient.getId(), Collections.emptySet(), true, false), messageDependsOnIds, recipient.getId().toQueueKey()); } else if (recipient.isDistributionList()) { jobManager.add(new PushDistributionListSendJob(messageId, recipient.getId(), true, Collections.emptySet()), messageDependsOnIds, recipient.getId().toQueueKey()); } else { - jobManager.add(IndividualSendJob.create(messageId, recipient, true), messageDependsOnIds, recipient.getId().toQueueKey()); + jobManager.add(IndividualSendJob.create(messageId, recipient, true, false), messageDependsOnIds, recipient.getId().toQueueKey()); } } } @@ -540,10 +540,10 @@ public class MessageSender { JobManager jobManager = ApplicationDependencies.getJobManager(); if (uploadJobIds.size() > 0) { - Job mediaSend = IndividualSendJob.create(messageId, recipient, true); + Job mediaSend = IndividualSendJob.create(messageId, recipient, true, false); jobManager.add(mediaSend, uploadJobIds); } else { - IndividualSendJob.enqueue(context, jobManager, messageId, recipient); + IndividualSendJob.enqueue(context, jobManager, messageId, recipient, false); } } @@ -551,10 +551,10 @@ public class MessageSender { JobManager jobManager = ApplicationDependencies.getJobManager(); if (uploadJobIds.size() > 0) { - Job groupSend = new PushGroupSendJob(messageId, recipient.getId(), filterRecipientIds, !uploadJobIds.isEmpty()); + Job groupSend = new PushGroupSendJob(messageId, recipient.getId(), filterRecipientIds, !uploadJobIds.isEmpty(), false); jobManager.add(groupSend, uploadJobIds, uploadJobIds.isEmpty() ? null : recipient.getId().toQueueKey()); } else { - PushGroupSendJob.enqueue(context, jobManager, messageId, recipient.getId(), filterRecipientIds); + PushGroupSendJob.enqueue(context, jobManager, messageId, recipient.getId(), filterRecipientIds, false); } }