From c25ce2bcdde8194cb47ccbbfc15a3c7fe84d52e8 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Fri, 24 Oct 2025 13:37:15 -0400 Subject: [PATCH] Fix bug related to in-memory jobs not being prioritized properly. --- .../securesms/jobs/FastJobStorage.kt | 13 +++++++--- .../securesms/jobs/FastJobStorageTest.kt | 26 +++++++++++++++++-- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/FastJobStorage.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/FastJobStorage.kt index 6886f98dd2..a4c8e0ebc3 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/FastJobStorage.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/FastJobStorage.kt @@ -356,10 +356,15 @@ class FastJobStorage(private val jobDatabase: JobDatabase) : JobStorage { mostEligibleJobForQueue.keys.removeAll(affectedQueues) for (queue in affectedQueues) { - jobDatabase.getMostEligibleJobInQueue(queue)?.let { - jobSpecCache[it.id] = it - placeJobInEligibleList(it.toMinimalJobSpec()) - } + minimalJobs + .filter { it.queueKey == queue } + .minWithOrNull( + compareByDescending { it.globalPriority } + .thenByDescending { it.queuePriority } + .thenBy { it.createTime } + .thenBy { it.id } + ) + ?.let { placeJobInEligibleList(it) } } for (jobId in ids) { diff --git a/app/src/test/java/org/thoughtcrime/securesms/jobs/FastJobStorageTest.kt b/app/src/test/java/org/thoughtcrime/securesms/jobs/FastJobStorageTest.kt index 0e772eb0d1..05dd332464 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/jobs/FastJobStorageTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/jobs/FastJobStorageTest.kt @@ -863,6 +863,28 @@ class FastJobStorageTest { verify(exactly = 0) { database.deleteJobs(ids) } } + @Test + fun `deleteJobs - memory-only job remains in mostEligibleJobForQueue after deleting other job in same queue`() { + // This test targets a case where we weren't handling in-memory jobs when calculating mostEligiibleForJobQueue after a deletion + val memoryJob = fullSpec(id = "memory-job", factoryKey = "f1", queueKey = "q1", isMemoryOnly = true, globalPriority = 10) + val durableJob = fullSpec(id = "durable-job", factoryKey = "f2", queueKey = "q1", isMemoryOnly = false, globalPriority = 5) + + val database = mockDatabase(listOf(memoryJob, durableJob)) + val subject = FastJobStorage(database) + subject.init() + + // Verify memory job is initially the most eligible + val initialNext = subject.getNextEligibleJob(System.currentTimeMillis(), NO_PREDICATE) + assertThat(initialNext).isNotNull().prop(JobSpec::id).isEqualTo("memory-job") + + // Delete the durable job + subject.deleteJobs(listOf("durable-job")) + + // The memory job should still be available as the most eligible + val afterDelete = subject.getNextEligibleJob(System.currentTimeMillis(), NO_PREDICATE) + assertThat(afterDelete).isNotNull().prop(JobSpec::id).isEqualTo("memory-job") + } + @Test fun `deleteJobs - deletes all relevant pieces`() { val subject = FastJobStorage(mockDatabase(DataSet1.FULL_SPECS)) @@ -1439,8 +1461,8 @@ class FastJobStorageTest { return mock } - private fun fullSpec(id: String, factoryKey: String, queueKey: String? = null): FullSpec { - return FullSpec(jobSpec(id, factoryKey, queueKey), emptyList(), emptyList()) + private fun fullSpec(id: String, factoryKey: String, queueKey: String? = null, isMemoryOnly: Boolean = false, globalPriority: Int = 0): FullSpec { + return FullSpec(jobSpec(id, factoryKey, queueKey, isMemoryOnly = isMemoryOnly, globalPriority = globalPriority), emptyList(), emptyList()) } private fun jobSpec(