From f4d6de466ba8d3449ddce38b1c2f7036c636d71f Mon Sep 17 00:00:00 2001 From: Cody Henthorne Date: Fri, 13 Aug 2021 13:33:56 -0400 Subject: [PATCH] Fix long SMS send with no service failure loop. --- .../securesms/jobs/SmsSendJob.java | 25 +++++------ .../securesms/jobs/SmsSentJob.java | 44 ++++++++++++------- .../service/SmsDeliveryListener.java | 11 ++--- 3 files changed, 46 insertions(+), 34 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/SmsSendJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/SmsSendJob.java index dd3339d4a..04b683f7c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/SmsSendJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/SmsSendJob.java @@ -36,8 +36,8 @@ public class SmsSendJob extends SendJob { private static final String KEY_MESSAGE_ID = "message_id"; private static final String KEY_RUN_ATTEMPT = "run_attempt"; - private long messageId; - private int runAttempt; + private final long messageId; + private final int runAttempt; public SmsSendJob(long messageId, @NonNull Recipient destination) { this(messageId, destination, 0); @@ -141,7 +141,7 @@ public class SmsSendJob extends SendJob { } ArrayList messages = SmsManager.getDefault().divideMessage(message.getBody()); - ArrayList sentIntents = constructSentIntents(message.getId(), message.getType(), messages, false); + ArrayList sentIntents = constructSentIntents(message.getId(), message.getType(), messages); ArrayList deliveredIntents = constructDeliveredIntents(message.getId(), message.getType(), messages); // NOTE 11/04/14 -- There's apparently a bug where for some unknown recipients @@ -171,14 +171,14 @@ public class SmsSendJob extends SendJob { } } - private ArrayList constructSentIntents(long messageId, long type, - ArrayList messages, boolean secure) + private ArrayList constructSentIntents(long messageId, long type, ArrayList messages) { ArrayList sentIntents = new ArrayList<>(messages.size()); + boolean isMultipart = messages.size() > 1; for (String ignored : messages) { sentIntents.add(PendingIntent.getBroadcast(context, 0, - constructSentIntent(context, messageId, type, secure, false), + constructSentIntent(context, messageId, type, isMultipart), 0)); } @@ -191,19 +191,18 @@ public class SmsSendJob extends SendJob { } ArrayList deliveredIntents = new ArrayList<>(messages.size()); + boolean isMultipart = messages.size() > 1; for (String ignored : messages) { deliveredIntents.add(PendingIntent.getBroadcast(context, 0, - constructDeliveredIntent(context, messageId, type), + constructDeliveredIntent(context, messageId, type, isMultipart), 0)); } return deliveredIntents; } - private Intent constructSentIntent(Context context, long messageId, long type, - boolean upgraded, boolean push) - { + private Intent constructSentIntent(Context context, long messageId, long type, boolean isMultipart) { Intent pending = new Intent(SmsDeliveryListener.SENT_SMS_ACTION, Uri.parse("custom://" + messageId + System.currentTimeMillis()), context, SmsDeliveryListener.class); @@ -211,18 +210,18 @@ public class SmsSendJob extends SendJob { pending.putExtra("type", type); pending.putExtra("message_id", messageId); pending.putExtra("run_attempt", Math.max(runAttempt, getRunAttempt())); - pending.putExtra("upgraded", upgraded); - pending.putExtra("push", push); + pending.putExtra("is_multipart", isMultipart); return pending; } - private Intent constructDeliveredIntent(Context context, long messageId, long type) { + private Intent constructDeliveredIntent(Context context, long messageId, long type, boolean isMultipart) { Intent pending = new Intent(SmsDeliveryListener.DELIVERED_SMS_ACTION, Uri.parse("custom://" + messageId + System.currentTimeMillis()), context, SmsDeliveryListener.class); pending.putExtra("type", type); pending.putExtra("message_id", messageId); + pending.putExtra("is_multipart", isMultipart); return pending; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/SmsSentJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/SmsSentJob.java index c186defb5..93918306b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/SmsSentJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/SmsSentJob.java @@ -21,36 +21,41 @@ public class SmsSentJob extends BaseJob { private static final String TAG = Log.tag(SmsSentJob.class); - private static final String KEY_MESSAGE_ID = "message_id"; - private static final String KEY_ACTION = "action"; - private static final String KEY_RESULT = "result"; - private static final String KEY_RUN_ATTEMPT = "run_attempt"; + private static final String KEY_MESSAGE_ID = "message_id"; + private static final String KEY_IS_MULTIPART = "is_multipart"; + private static final String KEY_ACTION = "action"; + private static final String KEY_RESULT = "result"; + private static final String KEY_RUN_ATTEMPT = "run_attempt"; - private long messageId; - private String action; - private int result; - private int runAttempt; + private final long messageId; + private final boolean isMultipart; + private final String action; + private final int result; + private final int runAttempt; - public SmsSentJob(long messageId, String action, int result, int runAttempt) { + public SmsSentJob(long messageId, boolean isMultipart, String action, int result, int runAttempt) { this(new Job.Parameters.Builder().build(), messageId, + isMultipart, action, result, runAttempt); } - private SmsSentJob(@NonNull Job.Parameters parameters, long messageId, String action, int result, int runAttempt) { + private SmsSentJob(@NonNull Job.Parameters parameters, long messageId, boolean isMultipart, String action, int result, int runAttempt) { super(parameters); - this.messageId = messageId; - this.action = action; - this.result = result; - this.runAttempt = runAttempt; + this.messageId = messageId; + this.isMultipart = isMultipart; + this.action = action; + this.result = result; + this.runAttempt = runAttempt; } @Override public @NonNull Data serialize() { return new Data.Builder().putLong(KEY_MESSAGE_ID, messageId) + .putBoolean(KEY_IS_MULTIPART, isMultipart) .putString(KEY_ACTION, action) .putInt(KEY_RESULT, result) .putInt(KEY_RUN_ATTEMPT, runAttempt) @@ -100,8 +105,14 @@ public class SmsSentJob extends BaseJob { break; case SmsManager.RESULT_ERROR_NO_SERVICE: case SmsManager.RESULT_ERROR_RADIO_OFF: - Log.w(TAG, "Service connectivity problem, requeuing..."); - ApplicationDependencies.getJobManager().add(new SmsSendJob(messageId, record.getIndividualRecipient(), runAttempt + 1)); + if (isMultipart) { + Log.w(TAG, "Service connectivity problem, but not retrying due to multipart"); + database.markAsSentFailed(messageId); + ApplicationDependencies.getMessageNotifier().notifyMessageDeliveryFailed(context, record.getRecipient(), record.getThreadId()); + } else { + Log.w(TAG, "Service connectivity problem, requeuing..."); + ApplicationDependencies.getJobManager().add(new SmsSendJob(messageId, record.getIndividualRecipient(), runAttempt + 1)); + } break; default: database.markAsSentFailed(messageId); @@ -117,6 +128,7 @@ public class SmsSentJob extends BaseJob { public @NonNull SmsSentJob create(@NonNull Parameters parameters, @NonNull Data data) { return new SmsSentJob(parameters, data.getLong(KEY_MESSAGE_ID), + data.getBooleanOrDefault(KEY_IS_MULTIPART, true), data.getString(KEY_ACTION), data.getInt(KEY_RESULT), data.getInt(KEY_RUN_ATTEMPT)); diff --git a/app/src/main/java/org/thoughtcrime/securesms/service/SmsDeliveryListener.java b/app/src/main/java/org/thoughtcrime/securesms/service/SmsDeliveryListener.java index 622762195..33766ab08 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/service/SmsDeliveryListener.java +++ b/app/src/main/java/org/thoughtcrime/securesms/service/SmsDeliveryListener.java @@ -20,15 +20,16 @@ public class SmsDeliveryListener extends BroadcastReceiver { @Override public void onReceive(Context context, Intent intent) { - JobManager jobManager = ApplicationDependencies.getJobManager(); - long messageId = intent.getLongExtra("message_id", -1); - int runAttempt = intent.getIntExtra("run_attempt", 0); + JobManager jobManager = ApplicationDependencies.getJobManager(); + long messageId = intent.getLongExtra("message_id", -1); + int runAttempt = intent.getIntExtra("run_attempt", 0); + boolean isMultipart = intent.getBooleanExtra("is_multipart", true); switch (intent.getAction()) { case SENT_SMS_ACTION: int result = getResultCode(); - jobManager.add(new SmsSentJob(messageId, SENT_SMS_ACTION, result, runAttempt)); + jobManager.add(new SmsSentJob(messageId, isMultipart, SENT_SMS_ACTION, result, runAttempt)); break; case DELIVERED_SMS_ACTION: byte[] pdu = intent.getByteArrayExtra("pdu"); @@ -59,7 +60,7 @@ public class SmsDeliveryListener extends BroadcastReceiver { else if (status >> 24 == 3) status = SmsDatabase.Status.STATUS_FAILED; } - jobManager.add(new SmsSentJob(messageId, DELIVERED_SMS_ACTION, status, runAttempt)); + jobManager.add(new SmsSentJob(messageId, isMultipart, DELIVERED_SMS_ACTION, status, runAttempt)); break; default: Log.w(TAG, "Unknown action: " + intent.getAction());