empathicqubit/Signal-Android
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

Prevent certain types of circular job dependencies.

Browse source
This commit is contained in:
Greyson Parrelli 2023-04-03 14:56:44 -04:00 • committed by Alex Hart
parent 2a9576baf5
commit bbdf54097e
4 changed files with 91 additions and 59 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

51
app/src/main/java/org/thoughtcrime/securesms/jobmanager/persistence/DependencySpec.java
View file

@ -1,51 +0,0 @@
package org.thoughtcrime.securesms.jobmanager.persistence;
import androidx.annotation.NonNull;
import java.util.Locale;
import java.util.Objects;
public final class DependencySpec {
private final String jobId;
private final String dependsOnJobId;
private final boolean memoryOnly;
public DependencySpec(@NonNull String jobId, @NonNull String dependsOnJobId, boolean memoryOnly) {
this.jobId = jobId;
this.dependsOnJobId = dependsOnJobId;
this.memoryOnly = memoryOnly;
}
public @NonNull String getJobId() {
return jobId;
}
public @NonNull String getDependsOnJobId() {
return dependsOnJobId;
}
public boolean isMemoryOnly() {
return memoryOnly;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
DependencySpec that = (DependencySpec) o;
return Objects.equals(jobId, that.jobId) &&
Objects.equals(dependsOnJobId, that.dependsOnJobId) &&
memoryOnly == that.memoryOnly;
}
@Override
public int hashCode() {
return Objects.hash(jobId, dependsOnJobId, memoryOnly);
}
@Override
public @NonNull String toString() {
return String.format(Locale.US, "jobSpecId: JOB::%s | dependsOnJobSpecId: JOB::%s | memoryOnly: %b", jobId, dependsOnJobId, memoryOnly);
}
}

13
app/src/main/java/org/thoughtcrime/securesms/jobmanager/persistence/DependencySpec.kt Normal file
View file

@ -0,0 +1,13 @@
package org.thoughtcrime.securesms.jobmanager.persistence
import java.util.Locale
class DependencySpec(
val jobId: String,
val dependsOnJobId: String,
val isMemoryOnly: Boolean
) {
override fun toString(): String {
return String.format(Locale.US, "jobSpecId: JOB::%s | dependsOnJobSpecId: JOB::%s | memoryOnly: %b", jobId, dependsOnJobId, isMemoryOnly)
}
}

32
app/src/main/java/org/thoughtcrime/securesms/jobs/FastJobStorage.kt
View file

@ -23,7 +23,7 @@ class FastJobStorage(private val jobDatabase: JobDatabase) : JobStorage {
jobConstraints += constraintSpec
}
for (dependencySpec in jobDatabase.allDependencySpecs) {
for (dependencySpec in jobDatabase.allDependencySpecs.filterNot { it.hasCircularDependency() }) {
val jobDependencies: MutableList<DependencySpec> = dependenciesByJobId.getOrPut(dependencySpec.jobId) { mutableListOf() }
jobDependencies += dependencySpec
}
@ -275,4 +275,34 @@ class FastJobStorage(private val jobDatabase: JobDatabase) : JobStorage {
private fun getJobById(id: String): JobSpec? {
return jobs.firstOrNull { it.id == id }
}
/**
* Note that this is currently only checking a specific kind of circular dependency -- ones that are
* created between dependencies and queues.
*
* More specifically, dependencies where one job depends on another job in the same queue that was
* scheduled *after* it. These dependencies will never resolve. Under normal circumstances these
* won't occur, but *could* occur if the user changed their clock (either purposefully or automatically).
*
* Rather than go through and delete them from the database, removing them from memory at load time
* serves the same effect and doesn't require new write methods. This should also be very rare.
*/
private fun DependencySpec.hasCircularDependency(): Boolean {
val job = getJobById(this.jobId)
val dependsOnJob = getJobById(this.dependsOnJobId)
if (job == null || dependsOnJob == null) {
return false
}
if (job.queueKey == null || dependsOnJob.queueKey == null) {
return false
}
if (job.queueKey != dependsOnJob.queueKey) {
return false
}
return dependsOnJob.createTime > job.createTime
}
}

54
app/src/test/java/org/thoughtcrime/securesms/jobs/FastJobStorageTest.java
View file

@ -39,6 +39,17 @@ public class FastJobStorageTest {
DataSet1.assertDependenciesMatch(subject.getAllDependencySpecs());
}
@Test
public void init_removesCircularDependencies() {
FastJobStorage subject = new FastJobStorage(fixedDataDatabase(DataSetCircularDependency.FULL_SPECS));
subject.init();
DataSetCircularDependency.assertJobsMatch(subject.getAllJobSpecs());
DataSetCircularDependency.assertConstraintsMatch(subject.getAllConstraintSpecs());
DataSetCircularDependency.assertDependenciesMatch(subject.getAllDependencySpecs());
}
@Test
public void insertJobs_writesToDatabase() {
JobDatabase database = noopDatabase();
@ -617,21 +628,21 @@ public class FastJobStorageTest {
static void assertJobsMatch(@NonNull List<JobSpec> jobs) {
assertEquals(jobs.size(), 3);
assertTrue(jobs.contains(DataSet1.JOB_1));
assertTrue(jobs.contains(DataSet1.JOB_1));
assertTrue(jobs.contains(DataSet1.JOB_3));
assertTrue(jobs.contains(JOB_1));
assertTrue(jobs.contains(JOB_2));
assertTrue(jobs.contains(JOB_3));
}
static void assertConstraintsMatch(@NonNull List<ConstraintSpec> constraints) {
assertEquals(constraints.size(), 2);
assertTrue(constraints.contains(DataSet1.CONSTRAINT_1));
assertTrue(constraints.contains(DataSet1.CONSTRAINT_2));
assertTrue(constraints.contains(CONSTRAINT_1));
assertTrue(constraints.contains(CONSTRAINT_2));
}
static void assertDependenciesMatch(@NonNull List<DependencySpec> dependencies) {
assertEquals(dependencies.size(), 2);
assertTrue(dependencies.contains(DataSet1.DEPENDENCY_2));
assertTrue(dependencies.contains(DataSet1.DEPENDENCY_3));
assertTrue(dependencies.contains(DEPENDENCY_2));
assertTrue(dependencies.contains(DEPENDENCY_3));
}
}
@ -641,4 +652,33 @@ public class FastJobStorageTest {
static final FullSpec FULL_SPEC_1 = new FullSpec(JOB_1, Collections.singletonList(CONSTRAINT_1), Collections.emptyList());
static final List<FullSpec> FULL_SPECS = Collections.singletonList(FULL_SPEC_1);
}
private static final class DataSetCircularDependency {
static final JobSpec JOB_1 = new JobSpec("id1", "f1", "q1", 1, 2, 3, 4, 5, null, null, false, false);
static final JobSpec JOB_2 = new JobSpec("id2", "f2", "q1", 2, 2, 3, 4, 5, null, null, false, false);
static final JobSpec JOB_3 = new JobSpec("id3", "f3", "q3", 3, 2, 3, 4, 5, null, null, false, false);
static final DependencySpec DEPENDENCY_1 = new DependencySpec("id1", "id2", false);
static final DependencySpec DEPENDENCY_3 = new DependencySpec("id3", "id2", false);
static final FullSpec FULL_SPEC_1 = new FullSpec(JOB_1, Collections.emptyList(), Collections.singletonList(DEPENDENCY_1));
static final FullSpec FULL_SPEC_2 = new FullSpec(JOB_2, Collections.emptyList(), Collections.emptyList());
static final FullSpec FULL_SPEC_3 = new FullSpec(JOB_3, Collections.emptyList(), Collections.singletonList(DEPENDENCY_3));
static final List<FullSpec> FULL_SPECS = Arrays.asList(FULL_SPEC_1, FULL_SPEC_2, FULL_SPEC_3);
static void assertJobsMatch(@NonNull List<JobSpec> jobs) {
assertEquals(jobs.size(), 3);
assertTrue(jobs.contains(JOB_1));
assertTrue(jobs.contains(JOB_2));
assertTrue(jobs.contains(JOB_3));
}
static void assertConstraintsMatch(@NonNull List<ConstraintSpec> constraints) {
assertEquals(constraints.size(), 0);
}
static void assertDependenciesMatch(@NonNull List<DependencySpec> dependencies) {
assertEquals(dependencies.size(), 1);
assertFalse(dependencies.contains(DEPENDENCY_1));
assertTrue(dependencies.contains(DEPENDENCY_3));
}
}
}