Remove CellServiceConstraint in favor of NetworkOrCellServiceConstraint.

If a job was enqueued with a CellServiceConstraint (which is currently
only SMS jobs), then it'll never run until it gets service, even if you
flip the "enable SMS sending over wifi" toggle.

This has created bad situations in the past, where SMS jobs just get
stuck on devices that never report having cell service (like VM's or
wifi only devices).

This fixes it by *always* using NetworkOrCellServiceConstraint, and then
deciding whether a constraint is met by checking the "wifi SMS" setting
at decision-time.
master
Greyson Parrelli 2020-05-01 12:44:53 -04:00
parent 0c2afa9438
commit 310ec8f296
7 changed files with 32 additions and 78 deletions

View File

@ -1416,8 +1416,7 @@ public class ConversationItem extends LinearLayout implements BindableConversati
database.markAsOutbox(messageRecord.getId());
database.markAsForcedSms(messageRecord.getId());
ApplicationDependencies.getJobManager().add(new SmsSendJob(context,
messageRecord.getId(),
ApplicationDependencies.getJobManager().add(new SmsSendJob(messageRecord.getId(),
messageRecord.getIndividualRecipient()));
}
});

View File

@ -1,51 +0,0 @@
package org.thoughtcrime.securesms.jobmanager.impl;
import android.app.Application;
import android.app.job.JobInfo;
import android.telephony.ServiceState;
import android.telephony.TelephonyManager;
import androidx.annotation.NonNull;
import org.thoughtcrime.securesms.jobmanager.Constraint;
import org.thoughtcrime.securesms.sms.TelephonyServiceState;
import org.thoughtcrime.securesms.util.ServiceUtil;
public class CellServiceConstraint implements Constraint {
public static final String KEY = "CellServiceConstraint";
private final Application application;
public CellServiceConstraint(@NonNull Application application) {
this.application = application;
}
@Override
public @NonNull String getFactoryKey() {
return KEY;
}
@Override
public boolean isMet() {
return CellServiceConstraintObserver.getInstance(application).hasService();
}
@Override
public void applyToJobInfo(@NonNull JobInfo.Builder jobInfoBuilder) {
}
public static final class Factory implements Constraint.Factory<CellServiceConstraint> {
private final Application application;
public Factory(@NonNull Application application) {
this.application = application;
}
@Override
public CellServiceConstraint create() {
return new CellServiceConstraint(application);
}
}
}

View File

@ -5,17 +5,19 @@ import android.app.job.JobInfo;
import androidx.annotation.NonNull;
import org.thoughtcrime.securesms.jobmanager.Constraint;
import org.thoughtcrime.securesms.util.TextSecurePreferences;
public class NetworkOrCellServiceConstraint implements Constraint {
public static final String KEY = "NetworkOrCellServiceConstraint";
public static final String KEY = "NetworkOrCellServiceConstraint";
public static final String LEGACY_KEY = "CellServiceConstraint";
private final NetworkConstraint networkConstraint;
private final CellServiceConstraint serviceConstraint;
private final Application application;
private final NetworkConstraint networkConstraint;
public NetworkOrCellServiceConstraint(@NonNull Application application) {
networkConstraint = new NetworkConstraint.Factory(application).create();
serviceConstraint = new CellServiceConstraint.Factory(application).create();
private NetworkOrCellServiceConstraint(@NonNull Application application) {
this.application = application;
this.networkConstraint = new NetworkConstraint.Factory(application).create();
}
@Override
@ -25,13 +27,21 @@ public class NetworkOrCellServiceConstraint implements Constraint {
@Override
public boolean isMet() {
return networkConstraint.isMet() || serviceConstraint.isMet();
if (TextSecurePreferences.isWifiSmsEnabled(application)) {
return networkConstraint.isMet() || hasCellService(application);
} else {
return hasCellService(application);
}
}
@Override
public void applyToJobInfo(@NonNull JobInfo.Builder jobInfoBuilder) {
}
private static boolean hasCellService(@NonNull Application application) {
return CellServiceConstraintObserver.getInstance(application).hasService();
}
public static class Factory implements Constraint.Factory<NetworkOrCellServiceConstraint> {
private final Application application;

View File

@ -9,7 +9,6 @@ import org.thoughtcrime.securesms.jobmanager.Constraint;
import org.thoughtcrime.securesms.jobmanager.ConstraintObserver;
import org.thoughtcrime.securesms.jobmanager.Job;
import org.thoughtcrime.securesms.jobmanager.JobMigration;
import org.thoughtcrime.securesms.jobmanager.impl.CellServiceConstraint;
import org.thoughtcrime.securesms.jobmanager.impl.CellServiceConstraintObserver;
import org.thoughtcrime.securesms.jobmanager.impl.NetworkConstraint;
import org.thoughtcrime.securesms.jobmanager.impl.NetworkConstraintObserver;
@ -142,10 +141,10 @@ public final class JobManagerFactories {
public static Map<String, Constraint.Factory> getConstraintFactories(@NonNull Application application) {
return new HashMap<String, Constraint.Factory>() {{
put(CellServiceConstraint.KEY, new CellServiceConstraint.Factory(application));
put(NetworkConstraint.KEY, new NetworkConstraint.Factory(application));
put(NetworkOrCellServiceConstraint.KEY, new NetworkOrCellServiceConstraint.Factory(application));
put(SqlCipherMigrationConstraint.KEY, new SqlCipherMigrationConstraint.Factory(application));
put(NetworkConstraint.KEY, new NetworkConstraint.Factory(application));
put(NetworkOrCellServiceConstraint.KEY, new NetworkOrCellServiceConstraint.Factory(application));
put(NetworkOrCellServiceConstraint.LEGACY_KEY, new NetworkOrCellServiceConstraint.Factory(application));
put(SqlCipherMigrationConstraint.KEY, new SqlCipherMigrationConstraint.Factory(application));
}};
}

View File

@ -12,7 +12,6 @@ import android.telephony.SmsManager;
import org.thoughtcrime.securesms.jobmanager.Data;
import org.thoughtcrime.securesms.jobmanager.Job;
import org.thoughtcrime.securesms.jobmanager.impl.NetworkOrCellServiceConstraint;
import org.thoughtcrime.securesms.jobmanager.impl.CellServiceConstraint;
import org.thoughtcrime.securesms.database.DatabaseFactory;
import org.thoughtcrime.securesms.database.NoSuchMessageException;
@ -39,12 +38,12 @@ public class SmsSendJob extends SendJob {
private long messageId;
private int runAttempt;
public SmsSendJob(Context context, long messageId, @NonNull Recipient destination) {
this(context, messageId, destination, 0);
public SmsSendJob(long messageId, @NonNull Recipient destination) {
this(messageId, destination, 0);
}
public SmsSendJob(Context context, long messageId, @NonNull Recipient destination, int runAttempt) {
this(constructParameters(context, destination), messageId, runAttempt);
public SmsSendJob(long messageId, @NonNull Recipient destination, int runAttempt) {
this(constructParameters(destination), messageId, runAttempt);
}
private SmsSendJob(@NonNull Job.Parameters parameters, long messageId, int runAttempt) {
@ -226,13 +225,11 @@ public class SmsSendJob extends SendJob {
}
}
private static Job.Parameters constructParameters(@NonNull Context context, @NonNull Recipient destination) {
String constraint = TextSecurePreferences.isWifiSmsEnabled(context) ? NetworkOrCellServiceConstraint.KEY
: CellServiceConstraint.KEY;
private static Job.Parameters constructParameters(@NonNull Recipient destination) {
return new Job.Parameters.Builder()
.setMaxAttempts(MAX_ATTEMPTS)
.setQueue(destination.getId().toQueueKey())
.addConstraint(constraint)
.addConstraint(NetworkOrCellServiceConstraint.KEY)
.build();
}

View File

@ -103,7 +103,7 @@ public class SmsSentJob extends BaseJob {
case SmsManager.RESULT_ERROR_NO_SERVICE:
case SmsManager.RESULT_ERROR_RADIO_OFF:
Log.w(TAG, "Service connectivity problem, requeuing...");
ApplicationDependencies.getJobManager().add(new SmsSendJob(context, messageId, record.getIndividualRecipient(), runAttempt + 1));
ApplicationDependencies.getJobManager().add(new SmsSendJob(messageId, record.getIndividualRecipient(), runAttempt + 1));
break;
default:
database.markAsSentFailed(messageId);

View File

@ -380,7 +380,7 @@ public class MessageSender {
} else if (!forceSms && isPushTextSend(context, recipient, keyExchange)) {
sendTextPush(recipient, messageId);
} else {
sendSms(context, recipient, messageId);
sendSms(recipient, messageId);
}
}
@ -411,9 +411,9 @@ public class MessageSender {
}
}
private static void sendSms(Context context, Recipient recipient, long messageId) {
private static void sendSms(Recipient recipient, long messageId) {
JobManager jobManager = ApplicationDependencies.getJobManager();
jobManager.add(new SmsSendJob(context, messageId, recipient));
jobManager.add(new SmsSendJob(messageId, recipient));
}
private static void sendMms(Context context, long messageId) {