From 8f51bdcb786b0e8e4ffc259ff22d6c3ce0a4254c Mon Sep 17 00:00:00 2001
From: AsamK <asamk@gmx.de>
Date: Sat, 6 Feb 2021 16:54:52 +0100
Subject: [PATCH] Adapt maxInstancesForQueue to only consider instances of the
 same job.

Currently the maxInstancesForQueue limit checks the count of all jobs in a
given queue. If there are already too many jobs, the new job is discarded.

However this is not the expected behavior for the two jobs where it's used:
GroupCallPeekWorkerJob and AutomaticSessionResetJob
For both the expected behavior is that there aren't too many jobs of them
started, but that there will be at least one instance of them started.
Both of them use the same queue as the PushProcessMessageJob and the MarkerJob.
Those two jobs are often in the queue at the same time, effectively preventing
the GroupCallPeekWorkerJob and AutomaticSessionResetJob from being enqueued.
---
 .../java/org/thoughtcrime/securesms/jobmanager/Job.java    | 4 ++--
 .../thoughtcrime/securesms/jobmanager/JobController.java   | 2 +-
 .../securesms/jobmanager/persistence/JobStorage.java       | 2 +-
 .../org/thoughtcrime/securesms/jobs/FastJobStorage.java    | 5 +++--
 .../thoughtcrime/securesms/jobs/FastJobStorageTest.java    | 7 ++++---
 5 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobmanager/Job.java b/app/src/main/java/org/thoughtcrime/securesms/jobmanager/Job.java
index 03bafa77eb..9e031a2fd3 100644
--- a/app/src/main/java/org/thoughtcrime/securesms/jobmanager/Job.java
+++ b/app/src/main/java/org/thoughtcrime/securesms/jobmanager/Job.java
@@ -407,8 +407,8 @@ public abstract class Job {
 
       /**
        * Specify the maximum number of instances you'd want of this job at any given time, as
-       * determined by the job's queue key. If enqueueing this job would put it over that limit,
-       * it will be ignored.
+       * determined by the job's factory key and queue key. If enqueueing this job would put it over
+       * that limit, it will be ignored.
        *
        * This property is ignored if the job is submitted as part of a {@link JobManager.Chain}, or
        * if the job has no queue key.
diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobmanager/JobController.java b/app/src/main/java/org/thoughtcrime/securesms/jobmanager/JobController.java
index 2e0e3becf4..014cecc10d 100644
--- a/app/src/main/java/org/thoughtcrime/securesms/jobmanager/JobController.java
+++ b/app/src/main/java/org/thoughtcrime/securesms/jobmanager/JobController.java
@@ -314,7 +314,7 @@ class JobController {
 
       boolean exceedsQueue   = solo.getParameters().getQueue() != null                                    &&
                                solo.getParameters().getMaxInstancesForQueue() != Job.Parameters.UNLIMITED &&
-                               jobStorage.getJobCountForQueue(solo.getParameters().getQueue()) >= solo.getParameters().getMaxInstancesForQueue();
+                               jobStorage.getJobCountForFactoryAndQueue(solo.getFactoryKey(), solo.getParameters().getQueue()) >= solo.getParameters().getMaxInstancesForQueue();
 
       if (exceedsQueue) {
         return true;
diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobmanager/persistence/JobStorage.java b/app/src/main/java/org/thoughtcrime/securesms/jobmanager/persistence/JobStorage.java
index 569e870ef3..904ce853ac 100644
--- a/app/src/main/java/org/thoughtcrime/securesms/jobmanager/persistence/JobStorage.java
+++ b/app/src/main/java/org/thoughtcrime/securesms/jobmanager/persistence/JobStorage.java
@@ -30,7 +30,7 @@ public interface JobStorage {
   int getJobCountForFactory(@NonNull String factoryKey);
 
   @WorkerThread
-  int getJobCountForQueue(@NonNull String queueKey);
+  int getJobCountForFactoryAndQueue(@NonNull String factoryKey, @NonNull String queueKey);
 
   @WorkerThread
   void updateJobRunningState(@NonNull String id, boolean isRunning);
diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/FastJobStorage.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/FastJobStorage.java
index 80bd6898e2..7c0d32b499 100644
--- a/app/src/main/java/org/thoughtcrime/securesms/jobs/FastJobStorage.java
+++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/FastJobStorage.java
@@ -167,9 +167,10 @@ public class FastJobStorage implements JobStorage {
   }
 
   @Override
-  public synchronized int getJobCountForQueue(@NonNull String queueKey) {
+  public synchronized int getJobCountForFactoryAndQueue(@NonNull String factoryKey, @NonNull String queueKey) {
     return (int) Stream.of(jobs)
-                       .filter(j -> queueKey.equals(j.getQueueKey()))
+                       .filter(j -> factoryKey.equals(j.getFactoryKey()) &&
+                                    queueKey.equals(j.getQueueKey()))
                        .count();
   }
 
diff --git a/app/src/test/java/org/thoughtcrime/securesms/jobs/FastJobStorageTest.java b/app/src/test/java/org/thoughtcrime/securesms/jobs/FastJobStorageTest.java
index 82c2957702..6b019fa4a3 100644
--- a/app/src/test/java/org/thoughtcrime/securesms/jobs/FastJobStorageTest.java
+++ b/app/src/test/java/org/thoughtcrime/securesms/jobs/FastJobStorageTest.java
@@ -547,13 +547,14 @@ public class FastJobStorageTest {
   }
 
   @Test
-  public void getJobCountForQueue_general() {
+  public void getJobCountForFactoryAndQueue_general() {
     FastJobStorage subject = new FastJobStorage(fixedDataDatabase(DataSet1.FULL_SPECS));
 
     subject.init();
 
-    assertEquals(1, subject.getJobCountForQueue("q1"));
-    assertEquals(0, subject.getJobCountForQueue("does-not-exist"));
+    assertEquals(1, subject.getJobCountForFactoryAndQueue("f1", "q1"));
+    assertEquals(0, subject.getJobCountForFactoryAndQueue("f2", "q1"));
+    assertEquals(0, subject.getJobCountForFactoryAndQueue("f1", "does-not-exist"));
   }
 
   private JobDatabase noopDatabase() {