From c46a005758dd0f4fda0d606df4b44c99b28575e2 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Tue, 18 Nov 2025 19:58:45 -0500 Subject: [PATCH] Fix bug in job eligibility sorting after a job deletion. --- .../securesms/jobs/FastJobStorage.kt | 1 - .../securesms/jobs/FastJobStorageTest.kt | 39 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) 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 a4c8e0ebc3..4e5be8af81 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/FastJobStorage.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/FastJobStorage.kt @@ -362,7 +362,6 @@ class FastJobStorage(private val jobDatabase: JobDatabase) : JobStorage { compareByDescending { it.globalPriority } .thenByDescending { it.queuePriority } .thenBy { it.createTime } - .thenBy { it.id } ) ?.let { placeJobInEligibleList(it) } } 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 05dd332464..6f827bf49a 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/jobs/FastJobStorageTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/jobs/FastJobStorageTest.kt @@ -829,6 +829,45 @@ class FastJobStorageTest { assertThat(subject.getNextEligibleJob(20, NO_PREDICATE)).isEqualTo(fullSpec1.jobSpec) } + @Test + fun `getNextEligibleJob - job chain with same createTime processes in correct order`() { + // Simulates a real job chain where all jobs are created in the same millisecond + // Jobs should process in dependency order, not alphabetical ID order + + val job1 = FullSpec( + jobSpec(id = "z-first", factoryKey = "f1", queueKey = "q1", createTime = 1000), + emptyList(), + emptyList() + ) + val job2 = FullSpec( + jobSpec(id = "m-second", factoryKey = "f2", queueKey = "q1", createTime = 1000), + emptyList(), + listOf(DependencySpec(jobId = "m-second", dependsOnJobId = "z-first", isMemoryOnly = false)) + ) + val job3 = FullSpec( + jobSpec(id = "a-third", factoryKey = "f3", queueKey = "q1", createTime = 1000), + emptyList(), + listOf(DependencySpec(jobId = "a-third", dependsOnJobId = "m-second", isMemoryOnly = false)) + ) + + val subject = FastJobStorage(mockDatabase(listOf(job1, job2, job3))) + subject.init() + + // First eligible should be job1 (no dependencies) + val first = subject.getNextEligibleJob(2000, NO_PREDICATE) + assertThat(first).isNotNull().prop(JobSpec::id).isEqualTo("z-first") + subject.deleteJob("z-first") + + // After deleting job1, job2 should be eligible (dependency resolved) + val second = subject.getNextEligibleJob(2000, NO_PREDICATE) + assertThat(second).isNotNull().prop(JobSpec::id).isEqualTo("m-second") + subject.deleteJob("m-second") + + // After deleting job2, job3 should be eligible + val third = subject.getNextEligibleJob(2000, NO_PREDICATE) + assertThat(third).isNotNull().prop(JobSpec::id).isEqualTo("a-third") + } + @Test fun `getEligibleJobCount - general`() { val subject = FastJobStorage(mockDatabase(DataSet1.FULL_SPECS))