From c57c31e887ed9a6abc6cf3b72b30dfc625cc3ce6 Mon Sep 17 00:00:00 2001 From: kalpesh patel <1988.kalpesh@gmail.com> Date: Sun, 29 Oct 2017 15:19:23 +0530 Subject: [PATCH 1/2] Added failing tests to check setting of Context and performing Injection before cancelling persistent job. --- .../jobmanager/ApplicationContextTests.java | 28 ++++++- .../test/jobmanager/InjectorTest.java | 76 ++++++++++++++++++- 2 files changed, 99 insertions(+), 5 deletions(-) diff --git a/jobqueue/src/test/java/com/birbit/android/jobqueue/test/jobmanager/ApplicationContextTests.java b/jobqueue/src/test/java/com/birbit/android/jobqueue/test/jobmanager/ApplicationContextTests.java index e42dea72..cfb06747 100644 --- a/jobqueue/src/test/java/com/birbit/android/jobqueue/test/jobmanager/ApplicationContextTests.java +++ b/jobqueue/src/test/java/com/birbit/android/jobqueue/test/jobmanager/ApplicationContextTests.java @@ -5,10 +5,12 @@ import android.support.annotation.Nullable; import com.birbit.android.jobqueue.CancelReason; +import com.birbit.android.jobqueue.CancelResult; import com.birbit.android.jobqueue.Job; import com.birbit.android.jobqueue.JobManager; import com.birbit.android.jobqueue.Params; import com.birbit.android.jobqueue.RetryConstraint; +import com.birbit.android.jobqueue.TagConstraint; import org.junit.Before; import org.junit.Test; @@ -29,6 +31,7 @@ public class ApplicationContextTests extends JobManagerTestBase { static int retryCount = 0; static List errors = new ArrayList<>(); + @Before public void clear() { retryCount = 0; @@ -45,6 +48,29 @@ public void getContextPersistent() throws InterruptedException, MultipleFailureE getContextTest(true); } + @Test + public void getContextCancelRequestPersistent() throws MultipleFailureException { + getContextCancelRequestTest(true); + } + + @Test + public void getContextCancelRequestNonPersistent() throws MultipleFailureException { + getContextCancelRequestTest(false); + } + + public void getContextCancelRequestTest(boolean persistent) throws MultipleFailureException { + JobManager jobManager = createJobManager(); + jobManager.stop(); + ContextCheckJob dummyJob = new ContextCheckJob(new Params(0).addTags("dummyTag").setPersistent(persistent)); + + jobManager.addJob(dummyJob); + CancelResult cancelResult = jobManager.cancelJobs(TagConstraint.ANY, "dummyTag"); + assertThat("Could not cancel job", cancelResult.getCancelledJobs().size() == 1); + if (!errors.isEmpty()) { + throw new MultipleFailureException(errors); + } + } + public void getContextTest(boolean persistent) throws InterruptedException, MultipleFailureException { final ContextCheckJob addedJob = new ContextCheckJob(new Params(1).setPersistent(persistent)); @@ -89,7 +115,7 @@ public void onAdded() { public void onRun() throws Throwable { assertContext("onRun"); if (retryCount < 2) { - retryCount ++; + retryCount++; throw new RuntimeException("failure on purpose"); } } diff --git a/jobqueue/src/test/java/com/birbit/android/jobqueue/test/jobmanager/InjectorTest.java b/jobqueue/src/test/java/com/birbit/android/jobqueue/test/jobmanager/InjectorTest.java index 166c8370..a9b57601 100644 --- a/jobqueue/src/test/java/com/birbit/android/jobqueue/test/jobmanager/InjectorTest.java +++ b/jobqueue/src/test/java/com/birbit/android/jobqueue/test/jobmanager/InjectorTest.java @@ -1,28 +1,46 @@ package com.birbit.android.jobqueue.test.jobmanager; +import android.support.annotation.NonNull; +import android.support.annotation.Nullable; + +import com.birbit.android.jobqueue.CancelReason; +import com.birbit.android.jobqueue.CancelResult; import com.birbit.android.jobqueue.Job; import com.birbit.android.jobqueue.JobHolder; import com.birbit.android.jobqueue.JobManager; import com.birbit.android.jobqueue.Params; +import com.birbit.android.jobqueue.RetryConstraint; +import com.birbit.android.jobqueue.TagConstraint; import com.birbit.android.jobqueue.config.Configuration; import com.birbit.android.jobqueue.di.DependencyInjector; import com.birbit.android.jobqueue.test.jobs.DummyJob; -import static org.hamcrest.CoreMatchers.*; -import org.hamcrest.*; + +import org.hamcrest.MatcherAssert; import org.junit.Test; import org.junit.runner.RunWith; -import org.robolectric.*; +import org.junit.runners.model.MultipleFailureException; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; import org.robolectric.annotation.Config; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.atomic.AtomicInteger; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.CoreMatchers.sameInstance; +import static org.hamcrest.MatcherAssert.assertThat; + @RunWith(RobolectricTestRunner.class) @Config(constants = com.birbit.android.jobqueue.BuildConfig.class) public class InjectorTest extends JobManagerTestBase { + static List errors = new ArrayList<>(); + static final JobManagerTestBase.ObjectReference injectedJobReference = new JobManagerTestBase.ObjectReference(); + @Test public void testInjector() throws Throwable { Configuration.Builder builder = new Configuration.Builder(RuntimeEnvironment.application); - final JobManagerTestBase.ObjectReference injectedJobReference = new JobManagerTestBase.ObjectReference(); final AtomicInteger injectionCallCount = new AtomicInteger(0); DependencyInjector dependencyInjector = new DependencyInjector() { @Override @@ -45,5 +63,55 @@ public void inject(Job job) { holder = nextJob(jobManager); MatcherAssert.assertThat("injection should be called for persistent job", holder.getJob(), equalTo(injectedJobReference.getObject())); MatcherAssert.assertThat("injection should be called two times for persistent job", injectionCallCount.get(), equalTo(3)); + + jobManager.addJob(new InjectionCheckJob(new Params(1).addTags("dummyNonPersistedTag"))); + CancelResult cancelResult = jobManager.cancelJobs(TagConstraint.ANY, "dummyNonPersistedTag"); + assertThat("Could not cancel non persistent job", cancelResult.getCancelledJobs().size() == 1); + MatcherAssert.assertThat("injection should be called only once for non persistent job", injectionCallCount.get(), equalTo(4)); + + jobManager.addJob(new InjectionCheckJob(new Params(1).addTags("dummyPersistedTag").persist())); + jobManager.addJob(new InjectionCheckJob(new Params(4))); //adding new job to override injection of above job. + cancelResult = jobManager.cancelJobs(TagConstraint.ANY, "dummyPersistedTag"); + assertThat("Could not cancel persistent job", cancelResult.getCancelledJobs().size() == 1); + for (Job job : cancelResult.getCancelledJobs()) { + MatcherAssert.assertThat("injection should be called for persistent job", job, equalTo(injectedJobReference.getObject())); + MatcherAssert.assertThat("injection should be called two times for persistent job", injectionCallCount.get(), equalTo(7)); + } + if (!errors.isEmpty()) { + throw new MultipleFailureException(errors); + } + } + + public static class InjectionCheckJob extends Job { + protected InjectionCheckJob(Params params) { + super(params); + } + + private void assertInjection(String method) { + try { + assertThat("Injected job instance should be same as job instance in " + method, + injectedJobReference.getObject(), sameInstance((Object) this)); + } catch (Throwable t) { + errors.add(t); + } + } + + @Override + public void onAdded() { + } + + @Override + public void onRun() throws Throwable { + } + + @Override + protected void onCancel(@CancelReason int cancelReason, @Nullable Throwable throwable) { + assertInjection("onCancel"); + } + + @Override + protected RetryConstraint shouldReRunOnThrowable(@NonNull Throwable throwable, int runCount, int maxRunCount) { + return RetryConstraint.CANCEL; + } } } From 79ca27110fc235350688b6c314611d82283cc1f3 Mon Sep 17 00:00:00 2001 From: kalpesh patel <1988.kalpesh@gmail.com> Date: Sun, 29 Oct 2017 15:23:39 +0530 Subject: [PATCH 2/2] Setting Context and performing Injection before cancelling persistent job. --- .../birbit/android/jobqueue/CancelHandler.java | 3 +++ .../android/jobqueue/JobManagerThread.java | 18 ++++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/jobqueue/src/main/java/com/birbit/android/jobqueue/CancelHandler.java b/jobqueue/src/main/java/com/birbit/android/jobqueue/CancelHandler.java index bb242ecf..2f26b359 100644 --- a/jobqueue/src/main/java/com/birbit/android/jobqueue/CancelHandler.java +++ b/jobqueue/src/main/java/com/birbit/android/jobqueue/CancelHandler.java @@ -54,6 +54,9 @@ void query(JobManagerThread jobManagerThread, ConsumerManager consumerManager) { void commit(JobManagerThread jobManagerThread) { for (JobHolder jobHolder : cancelled) { + if (jobHolder.persistent) { + jobManagerThread.setupJobHolder(jobHolder); + } try { jobHolder.onCancel(CancelReason.CANCELLED_WHILE_RUNNING); } catch (Throwable t) { diff --git a/jobqueue/src/main/java/com/birbit/android/jobqueue/JobManagerThread.java b/jobqueue/src/main/java/com/birbit/android/jobqueue/JobManagerThread.java index a20bcf04..60a9b553 100644 --- a/jobqueue/src/main/java/com/birbit/android/jobqueue/JobManagerThread.java +++ b/jobqueue/src/main/java/com/birbit/android/jobqueue/JobManagerThread.java @@ -150,11 +150,7 @@ private void handleAddJob(AddJobMessage message) { } else { JqLog.d("another job with same singleId: %s was already queued", job.getSingleInstanceId()); } - if(dependencyInjector != null) { - //inject members b4 calling onAdded - dependencyInjector.inject(job); - } - jobHolder.setApplicationContext(appContext); + setupJobHolder(jobHolder); jobHolder.getJob().onAdded(); callbackManager.notifyOnAdded(jobHolder.getJob()); if (insert) { @@ -669,10 +665,9 @@ JobHolder getNextJob(Collection runningJobGroups, boolean ignoreRunning) if (jobHolder == null) { return null; } - if (persistent && dependencyInjector != null) { - dependencyInjector.inject(jobHolder.getJob()); + if (persistent){ + setupJobHolder(jobHolder); } - jobHolder.setApplicationContext(appContext); jobHolder.setDeadlineIsReached(jobHolder.getDeadlineNs() <= now); if (jobHolder.getDeadlineNs() <= now && jobHolder.shouldCancelOnDeadline()) { @@ -683,4 +678,11 @@ JobHolder getNextJob(Collection runningJobGroups, boolean ignoreRunning) } return jobHolder; } + + protected void setupJobHolder(JobHolder jobHolder) { + jobHolder.setApplicationContext(appContext); + if (dependencyInjector != null) { + dependencyInjector.inject(jobHolder.getJob()); + } + } }