Let's have JobManager only deal with checked exceptions.

Also, switch to Builder for JobManager construction.
This commit is contained in:
Moxie Marlinspike 2014-11-11 21:11:57 -08:00
parent d9d4ec9d9d
commit 0d06d50a65
32 changed files with 180 additions and 94 deletions

View File

@ -7,6 +7,7 @@ import org.whispersystems.jobqueue.jobs.RequirementDeferringTestJob;
import org.whispersystems.jobqueue.jobs.RequirementTestJob;
import org.whispersystems.jobqueue.jobs.TestJob;
import org.whispersystems.jobqueue.persistence.JavaJobSerializer;
import org.whispersystems.jobqueue.requirements.RequirementProvider;
import org.whispersystems.jobqueue.util.MockRequirement;
import org.whispersystems.jobqueue.util.MockRequirementProvider;
import org.whispersystems.jobqueue.util.PersistentMockRequirement;
@ -15,12 +16,17 @@ import org.whispersystems.jobqueue.util.PersistentResult;
import org.whispersystems.jobqueue.util.RunnableThrowable;
import java.io.IOException;
import java.util.LinkedList;
import java.util.List;
public class JobManagerTest extends AndroidTestCase {
public void testTransientJobExecution() throws InterruptedException {
TestJob testJob = new TestJob();
JobManager jobManager = new JobManager(getContext(), "transient-test", null, null, 1);
JobManager jobManager = JobManager.newBuilder(getContext())
.withName("transient-test")
.withConsumerThreads(1)
.build();
jobManager.add(testJob);
@ -32,8 +38,12 @@ public class JobManagerTest extends AndroidTestCase {
MockRequirementProvider provider = new MockRequirementProvider();
MockRequirement requirement = new MockRequirement(false);
TestJob testJob = new RequirementTestJob(requirement);
JobManager jobManager = new JobManager(getContext(), "transient-requirement-test",
provider, null, 1);
JobManager jobManager = JobManager.newBuilder(getContext())
.withName("transient-requirement-test")
.withRequirementProviders(provider)
.withConsumerThreads(1)
.build();
jobManager.add(testJob);
@ -75,8 +85,12 @@ public class JobManagerTest extends AndroidTestCase {
MockRequirementProvider provider = new MockRequirementProvider();
MockRequirement requirement = new MockRequirement(false);
RequirementDeferringTestJob testJob = new RequirementDeferringTestJob(requirement, 5, waitRunnable);
JobManager jobManager = new JobManager(getContext(), "transient-requirement-test",
provider, null, 1);
JobManager jobManager = JobManager.newBuilder(getContext())
.withName("transient-requirement-test")
.withRequirementProviders(provider)
.withConsumerThreads(1)
.build();
jobManager.add(testJob);
@ -106,8 +120,11 @@ public class JobManagerTest extends AndroidTestCase {
public void testPersistentJobExecuton() throws InterruptedException {
PersistentMockRequirement requirement = new PersistentMockRequirement();
PersistentTestJob testJob = new PersistentTestJob(requirement);
JobManager jobManager = new JobManager(getContext(), "persistent-requirement-test3",
null, new JavaJobSerializer(getContext()), 1);
JobManager jobManager = JobManager.newBuilder(getContext())
.withName("persistent-requirement-test3")
.withJobSerializer(new JavaJobSerializer(getContext()))
.withConsumerThreads(1)
.build();
PersistentResult.getInstance().reset();
PersistentRequirement.getInstance().setPresent(false);
@ -118,8 +135,12 @@ public class JobManagerTest extends AndroidTestCase {
assertTrue(!PersistentResult.getInstance().isRan());
PersistentRequirement.getInstance().setPresent(true);
jobManager = new JobManager(getContext(), "persistent-requirement-test3", null,
new JavaJobSerializer(getContext()), 1);
jobManager = JobManager.newBuilder(getContext())
.withName("persistent-requirement-test3")
.withJobSerializer(new JavaJobSerializer(getContext()))
.withConsumerThreads(1)
.build();
assertTrue(PersistentResult.getInstance().isRan());
}
@ -128,8 +149,12 @@ public class JobManagerTest extends AndroidTestCase {
EncryptionKeys keys = new EncryptionKeys(new byte[30]);
PersistentMockRequirement requirement = new PersistentMockRequirement();
PersistentTestJob testJob = new PersistentTestJob(requirement, keys);
JobManager jobManager = new JobManager(getContext(), "persistent-requirement-test4",
null, new JavaJobSerializer(getContext()), 1);
JobManager jobManager = JobManager.newBuilder(getContext())
.withName("persistent-requirement-test4")
.withJobSerializer(new JavaJobSerializer(getContext()))
.withConsumerThreads(1)
.build();
jobManager.setEncryptionKeys(keys);
PersistentResult.getInstance().reset();
@ -141,7 +166,11 @@ public class JobManagerTest extends AndroidTestCase {
assertTrue(!PersistentResult.getInstance().isRan());
PersistentRequirement.getInstance().setPresent(true);
jobManager = new JobManager(getContext(), "persistent-requirement-test4", null, new JavaJobSerializer(getContext()), 1);
jobManager = JobManager.newBuilder(getContext())
.withName("persistent-requirement-test4")
.withJobSerializer(new JavaJobSerializer(getContext()))
.withConsumerThreads(1)
.build();
assertTrue(!PersistentResult.getInstance().isRan());
@ -169,7 +198,10 @@ public class JobManagerTest extends AndroidTestCase {
TestJob testJobOne = new TestJob(JobParameters.newBuilder().withGroupId("foo").create(), waitRunnable);
TestJob testJobTwo = new TestJob(JobParameters.newBuilder().withGroupId("foo").create());
TestJob testJobThree = new TestJob(JobParameters.newBuilder().withGroupId("bar").create());
JobManager jobManager = new JobManager(getContext(), "transient-test", null, null, 3);
JobManager jobManager = JobManager.newBuilder(getContext())
.withName("transient-test")
.withConsumerThreads(3)
.build();
jobManager.add(testJobOne);
jobManager.add(testJobTwo);

View File

@ -19,11 +19,11 @@ public class PersistentTestJob extends Job {
@Override
public void onAdded() {
PersistentResult.getInstance().onAdded();;
PersistentResult.getInstance().onAdded();
}
@Override
public void onRun() throws Throwable {
public void onRun() throws Exception {
PersistentResult.getInstance().onRun();
}
@ -33,7 +33,7 @@ public class PersistentTestJob extends Job {
}
@Override
public boolean onShouldRetry(Throwable throwable) {
public boolean onShouldRetry(Exception exception) {
return false;
}
}

View File

@ -20,7 +20,7 @@ public class RequirementDeferringTestJob extends TestJob {
}
@Override
public void onRun() throws Throwable {
public void onRun() throws Exception {
synchronized (RAN_LOCK) {
this.ran = true;
}
@ -34,8 +34,8 @@ public class RequirementDeferringTestJob extends TestJob {
}
@Override
public boolean onShouldRetry(Throwable throwable) {
if (throwable instanceof Exception) {
public boolean onShouldRetry(Exception exception) {
if (exception instanceof Exception) {
return true;
}
return false;

View File

@ -37,7 +37,7 @@ public class TestJob extends Job {
}
@Override
public void onRun() throws Throwable {
public void onRun() throws Exception {
synchronized (RAN_LOCK) {
this.ran = true;
}
@ -54,7 +54,7 @@ public class TestJob extends Job {
}
@Override
public boolean onShouldRetry(Throwable throwable) {
public boolean onShouldRetry(Exception exception) {
return false;
}

View File

@ -15,4 +15,9 @@ public class MockRequirementProvider implements RequirementProvider {
public void setListener(RequirementListener listener) {
this.listener = listener;
}
@Override
public String getName() {
return "mock-requirement-provider";
}
}

View File

@ -23,7 +23,7 @@ public class PersistentResult {
}
}
public void onRun() throws Throwable {
public void onRun() throws Exception {
synchronized (RAN_LOCK) {
this.ran = true;
}

View File

@ -2,7 +2,7 @@ package org.whispersystems.jobqueue.util;
public interface RunnableThrowable {
public void run() throws Throwable;
public void run() throws Exception;
public void shouldThrow(Boolean value);
}

View File

@ -81,9 +81,10 @@ public abstract class Job implements Serializable {
}
public abstract void onAdded();
public abstract void onRun() throws Throwable;
public abstract void onRun() throws Exception;
public abstract boolean onShouldRetry(Exception exception);
public abstract void onCanceled();
public abstract boolean onShouldRetry(Throwable throwable);
}

View File

@ -42,11 +42,12 @@ public class JobConsumer extends Thread {
@Override
public void run() {
while (true) {
Job job = jobQueue.getNext();
Job job = jobQueue.getNext();
JobResult result = runJob(job);
JobResult result;
if ((result = runJob(job)) != JobResult.DEFERRED) {
if (result == JobResult.DEFERRED) {
jobQueue.push(job);
} else {
if (result == JobResult.FAILURE) {
job.onCanceled();
}
@ -54,8 +55,6 @@ public class JobConsumer extends Thread {
if (job.isPersistent()) {
persistentStorage.remove(job.getPersistentId());
}
} else {
jobQueue.add(job);
}
if (job.getGroupId() != null) {
@ -72,9 +71,11 @@ public class JobConsumer extends Thread {
try {
job.onRun();
return JobResult.SUCCESS;
} catch (Throwable throwable) {
Log.w(TAG, throwable);
if (!job.onShouldRetry(throwable)) {
} catch (Exception exception) {
Log.w(TAG, exception);
if (exception instanceof RuntimeException) {
throw (RuntimeException)exception;
} else if (!job.onShouldRetry(exception)) {
return JobResult.FAILURE;
} else if (!job.isRequirementsMet()) {
job.setRunIteration(runIteration+1);

View File

@ -26,6 +26,7 @@ import org.whispersystems.jobqueue.requirements.RequirementListener;
import org.whispersystems.jobqueue.requirements.RequirementProvider;
import java.io.IOException;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
@ -41,10 +42,10 @@ public class JobManager implements RequirementListener {
private final List<RequirementProvider> requirementProviders;
private final DependencyInjector dependencyInjector;
public JobManager(Context context, String name,
List<RequirementProvider> requirementProviders,
DependencyInjector dependencyInjector,
JobSerializer jobSerializer, int consumers)
private JobManager(Context context, String name,
List<RequirementProvider> requirementProviders,
DependencyInjector dependencyInjector,
JobSerializer jobSerializer, int consumers)
{
this.persistentStorage = new PersistentStorage(context, name, jobSerializer, dependencyInjector);
this.requirementProviders = requirementProviders;
@ -63,6 +64,10 @@ public class JobManager implements RequirementListener {
}
}
public static Builder newBuilder(Context context) {
return new Builder(context);
}
public RequirementProvider getRequirementProvider(String name) {
for (RequirementProvider provider : requirementProviders) {
if (provider.getName().equals(name)) {
@ -131,4 +136,53 @@ public class JobManager implements RequirementListener {
}
}
public static class Builder {
private final Context context;
private String name;
private List<RequirementProvider> requirementProviders;
private DependencyInjector dependencyInjector;
private JobSerializer jobSerializer;
private int consumerThreads;
Builder(Context context) {
this.context = context;
this.consumerThreads = 5;
}
public Builder withName(String name) {
this.name = name;
return this;
}
public Builder withRequirementProviders(RequirementProvider... requirementProviders) {
this.requirementProviders = Arrays.asList(requirementProviders);
return this;
}
public Builder withDependencyInjector(DependencyInjector dependencyInjector) {
this.dependencyInjector = dependencyInjector;
return this;
}
public Builder withJobSerializer(JobSerializer jobSerializer) {
this.jobSerializer = jobSerializer;
return this;
}
public Builder withConsumerThreads(int consumerThreads) {
this.consumerThreads = consumerThreads;
return this;
}
public JobManager build() {
if (name == null) {
throw new IllegalArgumentException("You must specify a name!");
}
return new JobManager(context, name, requirementProviders,
dependencyInjector, jobSerializer,
consumerThreads);
}
}
}

View File

@ -43,6 +43,10 @@ public class JobQueue {
notifyAll();
}
synchronized void push(Job job) {
jobQueue.push(job);
}
public synchronized Job getNext() {
try {
Job nextAvailableJob;

View File

@ -22,18 +22,12 @@ import android.content.Context;
import org.thoughtcrime.securesms.crypto.PRNGFixes;
import org.thoughtcrime.securesms.dependencies.AxolotlStorageModule;
import org.thoughtcrime.securesms.dependencies.InjectableType;
import org.thoughtcrime.securesms.jobs.persistence.EncryptingJobSerializer;
import org.thoughtcrime.securesms.jobs.GcmRefreshJob;
import org.thoughtcrime.securesms.jobs.requirements.MasterSecretRequirementProvider;
import org.thoughtcrime.securesms.dependencies.TextSecureCommunicationModule;
import org.thoughtcrime.securesms.jobs.GcmRefreshJob;
import org.thoughtcrime.securesms.jobs.persistence.EncryptingJobSerializer;
import org.thoughtcrime.securesms.util.TextSecurePreferences;
import org.whispersystems.jobqueue.JobManager;
import org.whispersystems.jobqueue.dependencies.DependencyInjector;
import org.whispersystems.jobqueue.requirements.NetworkRequirementProvider;
import org.whispersystems.jobqueue.requirements.RequirementProvider;
import java.util.LinkedList;
import java.util.List;
import dagger.ObjectGraph;
@ -79,13 +73,12 @@ public class ApplicationContext extends Application implements DependencyInjecto
}
private void initializeJobManager() {
List<RequirementProvider> providers = new LinkedList<RequirementProvider>() {{
add(new NetworkRequirementProvider(ApplicationContext.this));
add(new MasterSecretRequirementProvider(ApplicationContext.this));
}};
this.jobManager = new JobManager(this, "TextSecureJobs", providers, this,
new EncryptingJobSerializer(this), 5);
this.jobManager = JobManager.newBuilder(this)
.withName("TextSecureJobs")
.withDependencyInjector(this)
.withJobSerializer(new EncryptingJobSerializer(this))
.withConsumerThreads(5)
.build();
}
private void initializeDependencyInjection() {

View File

@ -77,8 +77,8 @@ public class AttachmentDownloadJob extends MasterSecretJob implements Injectable
}
@Override
public boolean onShouldRetryThrowable(Throwable throwable) {
if (throwable instanceof PushNetworkException) return true;
public boolean onShouldRetryThrowable(Exception exception) {
if (exception instanceof PushNetworkException) return true;
return false;
}

View File

@ -91,8 +91,8 @@ public class AvatarDownloadJob extends MasterSecretJob {
public void onCanceled() {}
@Override
public boolean onShouldRetryThrowable(Throwable throwable) {
if (throwable instanceof IOException) return true;
public boolean onShouldRetryThrowable(Exception exception) {
if (exception instanceof IOException) return true;
return false;
}

View File

@ -84,7 +84,7 @@ public class CleanPreKeysJob extends MasterSecretJob implements InjectableType {
}
@Override
public boolean onShouldRetryThrowable(Throwable throwable) {
public boolean onShouldRetryThrowable(Exception throwable) {
if (throwable instanceof NonSuccessfulResponseCodeException) return false;
if (throwable instanceof PushNetworkException) return true;
return false;

View File

@ -7,7 +7,6 @@ import org.thoughtcrime.securesms.crypto.IdentityKeyUtil;
import org.thoughtcrime.securesms.crypto.MasterSecret;
import org.thoughtcrime.securesms.crypto.PreKeyUtil;
import org.thoughtcrime.securesms.dependencies.InjectableType;
import org.thoughtcrime.securesms.push.TextSecureCommunicationFactory;
import org.thoughtcrime.securesms.util.ParcelUtil;
import org.thoughtcrime.securesms.util.TextSecurePreferences;
import org.whispersystems.jobqueue.EncryptionKeys;
@ -60,8 +59,8 @@ public class CreateSignedPreKeyJob extends ContextJob implements InjectableType
public void onCanceled() {}
@Override
public boolean onShouldRetry(Throwable throwable) {
if (throwable instanceof PushNetworkException) return true;
public boolean onShouldRetry(Exception exception) {
if (exception instanceof PushNetworkException) return true;
return false;
}
}

View File

@ -62,10 +62,10 @@ public class DeliveryReceiptJob extends ContextJob implements InjectableType {
}
@Override
public boolean onShouldRetry(Throwable throwable) {
Log.w(TAG, throwable);
if (throwable instanceof NonSuccessfulResponseCodeException) return false;
if (throwable instanceof PushNetworkException) return true;
public boolean onShouldRetry(Exception exception) {
Log.w(TAG, exception);
if (exception instanceof NonSuccessfulResponseCodeException) return false;
if (exception instanceof PushNetworkException) return true;
return false;
}

View File

@ -56,7 +56,7 @@ public class GcmRefreshJob extends ContextJob {
}
@Override
public boolean onShouldRetry(Throwable throwable) {
public boolean onShouldRetry(Exception throwable) {
if (throwable instanceof NonSuccessfulResponseCodeException) return false;
return true;
}

View File

@ -13,19 +13,19 @@ public abstract class MasterSecretJob extends ContextJob {
}
@Override
public void onRun() throws Throwable {
public void onRun() throws Exception {
MasterSecret masterSecret = getMasterSecret();
onRun(masterSecret);
}
@Override
public boolean onShouldRetry(Throwable throwable) {
if (throwable instanceof RequirementNotMetException) return true;
return onShouldRetryThrowable(throwable);
public boolean onShouldRetry(Exception exception) {
if (exception instanceof RequirementNotMetException) return true;
return onShouldRetryThrowable(exception);
}
public abstract void onRun(MasterSecret masterSecret) throws Throwable;
public abstract boolean onShouldRetryThrowable(Throwable throwable);
public abstract void onRun(MasterSecret masterSecret) throws Exception;
public abstract boolean onShouldRetryThrowable(Exception exception);
private MasterSecret getMasterSecret() throws RequirementNotMetException {
MasterSecret masterSecret = KeyCachingService.getMasterSecret(context);

View File

@ -179,7 +179,7 @@ public class MmsDownloadJob extends MasterSecretJob {
}
@Override
public boolean onShouldRetryThrowable(Throwable throwable) {
public boolean onShouldRetryThrowable(Exception exception) {
return false;
}

View File

@ -63,7 +63,7 @@ public class MmsReceiveJob extends ContextJob {
}
@Override
public boolean onShouldRetry(Throwable throwable) {
public boolean onShouldRetry(Exception exception) {
return false;
}
}

View File

@ -84,7 +84,7 @@ public class MmsSendJob extends MasterSecretJob {
}
@Override
public boolean onShouldRetryThrowable(Throwable throwable) {
public boolean onShouldRetryThrowable(Exception exception) {
return false;
}

View File

@ -78,7 +78,7 @@ public class PushDecryptJob extends MasterSecretJob {
}
@Override
public boolean onShouldRetryThrowable(Throwable throwable) {
public boolean onShouldRetryThrowable(Exception exception) {
return false;
}

View File

@ -97,8 +97,8 @@ public class PushGroupSendJob extends PushSendJob implements InjectableType {
}
@Override
public boolean onShouldRetryThrowable(Throwable throwable) {
if (throwable instanceof IOException) return true;
public boolean onShouldRetryThrowable(Exception exception) {
if (exception instanceof IOException) return true;
return false;
}

View File

@ -86,8 +86,8 @@ public class PushMediaSendJob extends PushSendJob implements InjectableType {
}
@Override
public boolean onShouldRetryThrowable(Throwable throwable) {
if (throwable instanceof RequirementNotMetException) return true;
public boolean onShouldRetryThrowable(Exception exception) {
if (exception instanceof RequirementNotMetException) return true;
return false;
}

View File

@ -60,7 +60,7 @@ public class PushReceiveJob extends ContextJob {
}
@Override
public boolean onShouldRetry(Throwable throwable) {
public boolean onShouldRetry(Exception exception) {
return false;
}

View File

@ -82,8 +82,8 @@ public class PushTextSendJob extends PushSendJob implements InjectableType {
}
@Override
public boolean onShouldRetryThrowable(Throwable throwable) {
if (throwable instanceof RetryLaterException) return true;
public boolean onShouldRetryThrowable(Exception exception) {
if (exception instanceof RetryLaterException) return true;
return false;
}

View File

@ -9,7 +9,6 @@ import org.thoughtcrime.securesms.crypto.MasterSecret;
import org.thoughtcrime.securesms.crypto.PreKeyUtil;
import org.thoughtcrime.securesms.dependencies.InjectableType;
import org.thoughtcrime.securesms.jobs.requirements.MasterSecretRequirement;
import org.thoughtcrime.securesms.push.TextSecureCommunicationFactory;
import org.thoughtcrime.securesms.util.TextSecurePreferences;
import org.whispersystems.jobqueue.JobParameters;
import org.whispersystems.jobqueue.requirements.NetworkRequirement;
@ -17,7 +16,6 @@ import org.whispersystems.libaxolotl.IdentityKeyPair;
import org.whispersystems.libaxolotl.state.PreKeyRecord;
import org.whispersystems.libaxolotl.state.SignedPreKeyRecord;
import org.whispersystems.textsecure.api.TextSecureAccountManager;
import org.whispersystems.textsecure.push.PushServiceSocket;
import org.whispersystems.textsecure.push.exceptions.NonSuccessfulResponseCodeException;
import org.whispersystems.textsecure.push.exceptions.PushNetworkException;
@ -76,9 +74,9 @@ public class RefreshPreKeysJob extends MasterSecretJob implements InjectableType
}
@Override
public boolean onShouldRetryThrowable(Throwable throwable) {
if (throwable instanceof NonSuccessfulResponseCodeException) return false;
if (throwable instanceof PushNetworkException) return true;
public boolean onShouldRetryThrowable(Exception exception) {
if (exception instanceof NonSuccessfulResponseCodeException) return false;
if (exception instanceof PushNetworkException) return true;
return false;
}

View File

@ -5,15 +5,14 @@ import android.util.Log;
import org.thoughtcrime.securesms.crypto.AsymmetricMasterCipher;
import org.thoughtcrime.securesms.crypto.AsymmetricMasterSecret;
import org.thoughtcrime.securesms.crypto.SecurityEvent;
import org.thoughtcrime.securesms.crypto.MasterSecret;
import org.thoughtcrime.securesms.crypto.MasterSecretUtil;
import org.thoughtcrime.securesms.crypto.SecurityEvent;
import org.thoughtcrime.securesms.crypto.SmsCipher;
import org.thoughtcrime.securesms.crypto.storage.TextSecureAxolotlStore;
import org.thoughtcrime.securesms.database.DatabaseFactory;
import org.thoughtcrime.securesms.database.EncryptingSmsDatabase;
import org.thoughtcrime.securesms.database.NoSuchMessageException;
import org.thoughtcrime.securesms.database.SmsDatabase;
import org.thoughtcrime.securesms.database.model.SmsMessageRecord;
import org.thoughtcrime.securesms.jobs.requirements.MasterSecretRequirement;
import org.thoughtcrime.securesms.notifications.MessageNotifier;
@ -94,7 +93,7 @@ public class SmsDecryptJob extends MasterSecretJob {
}
@Override
public boolean onShouldRetryThrowable(Throwable throwable) {
public boolean onShouldRetryThrowable(Exception exception) {
return false;
}

View File

@ -55,7 +55,7 @@ public class SmsReceiveJob extends ContextJob {
}
@Override
public boolean onShouldRetry(Throwable throwable) {
public boolean onShouldRetry(Exception exception) {
return false;
}

View File

@ -73,7 +73,7 @@ public class SmsSendJob extends MasterSecretJob {
}
@Override
public boolean onShouldRetryThrowable(Throwable throwable) {
public boolean onShouldRetryThrowable(Exception throwable) {
return false;
}

View File

@ -58,7 +58,7 @@ public class SmsSentJob extends MasterSecretJob {
}
@Override
public boolean onShouldRetryThrowable(Throwable throwable) {
public boolean onShouldRetryThrowable(Exception throwable) {
return false;
}