Reattach to conversions and joins the ViewModel did not start
The queue surviving process death is the stated reason this app uses WorkManager, and
the ViewModel was where that protection stopped. `activeWorkId` and `observer` are
plain fields, so a process reclaimed after a conversion finished came back to Idle
while the output sat in `cacheDir` with nothing in the UI able to reach it. The
realistic window is not a crash mid-transcode -- it is the job finishing, the user not
saving yet, and the process being reclaimed hours later as an ordinary background one.
Both ViewModels now query their own worker's class name on init and pick up what they
find. Nothing is persisted for it, and nothing needed to be: `WorkRequest.Builder`
seeds every request's tag set with `workerClass.name` (`tags = mutableSetOf(
workerClass.name)`, work-runtime 2.11.2), and R8 keeps those names through
work-runtime's own consumer rule, `-keepnames class * extends
androidx.work.ListenableWorker`. A UUID in a `SavedStateHandle` would have been both
more machinery and less: it cannot find work enqueued by a previous install.
What the query cannot return is the job's input. `WorkInfo` hands back id, state, tags,
progress, output and run-attempt count -- never the `Data` a request was enqueued with
-- so a ViewModel could see that a conversion existed and where its output went, but
not which file it was converting. The display name and size therefore ride on tags too,
which is the whole of the production change to the request builders. The picked `Uri`
deliberately does not: nothing in a reattached state reads it, and a `content://` grant
taken by a picker in a process that no longer exists is not something to hand back as
though it still worked.
The decision is a pure function on the JVM test stack, following `FailureOutcome`:
`Reattachment.choose` takes what WorkManager reported and answers which job, if any.
Cancelled work is excluded -- the user already said no. Failed work is excluded, which
matters more than it looks now that a device has shown an interrupted worker coming
back FAILED rather than retried, its restart's `setForeground` refused as a background
foreground-service start: nothing marks a failure as seen, so it would otherwise
reappear on every launch. A success whose staged file is gone is excluded, because a
Save button that fails on tap is worse than no button. Live work outranks a finished
result, since a running job holds a foreground notification and someone opening the app
while that notification is in the shade is looking for that conversion.
Where several jobs qualify, the newest staged file wins, and that is not a detail.
Losing a tie is not the same as waiting for the next launch: the tag query has no
`ORDER BY`, so its order is unspecified but stable, and an arbitrary winner would keep
winning every launch while the other result stayed unreachable for as long as its file
existed. It is reachable today -- dismiss one result with "Start over", which leaves its
file behind, then convert something else and do not save it. `WorkInfo` carries no
timestamp of any kind, but the edge is already stat'ing the file, so the file's own
mtime is the ordering. A clock that moves backwards makes it a heuristic; an order that
is unspecified and repeats itself is worse.
Aliases are the tie that does not resolve, and that case is not hypothetical. A tag
query on a device returned two SUCCEEDED jobs whose output paths were both
`.../conversions/input_converted.mp4`, with one file on disk -- the staging name is
derived from the input's display name, so a later job overwrites an earlier one's output
and both go on reporting it. Checking the file does not separate them, and neither does
its mtime, since they share it. So the two halves are separated instead. The file is
offered, because it is the user's file either way and losing it is the defect being
fixed; the label is not, because saying which job produced it would be a guess.
`Reattachment.Ambiguous` says so and the card falls back to a neutral name rather than
borrowing the other job's. Aliases whose tags are identical -- the ordinary case, the
same file converted twice -- stay attributed, since nothing turns on which wrote it.
Age is not filtered on, and cannot be: WorkManager keeps finished work about a week and
prunes on its own schedule, so a query in a fresh process routinely returns jobs from
earlier sessions. Whether the staged file is still there is the only signal separating a
result still worth offering from one already dealt with, which is why that check carries
the weight.
Two smaller behaviours fall out of reattaching rather than starting:
- A reattached job that is cancelled lands on Idle rather than Ready. Ready would put
a Convert button over an input URI that belongs to a dead process. `observe()` takes
the cancelled destination as a defaulted parameter, so a job started here is
unchanged.
- A FAILED job's message falls back when blank, not only when absent. A worker killed
before it can report leaves no output data at all, and an exception's message can be
the empty string; both used to reach the screen as a failure with nothing said.
Deliberately not fixed here, each being its own change: the save dialog's suggested name
and MIME still come from the current picker rather than from the job that ran, so a
reattached job in a non-default format is offered the default extension; a result
dismissed with "Start over" still keeps its staged file, so it can be offered again next
launch -- the file check closes that for free once the file is deleted; and
`setForeground` still sits outside `doWork`'s try, so its throw bypasses the retry
decision entirely.
Tested where it can be. 28 JVM tests cover the choice and the tag round trip, including
the ordering, the aliasing rules and a display name that looks like another tag. The
reattachment itself is instrumented: a ViewModel constructed against the real
WorkManager is the next launch, with no memory of the work. `WorkManagerTestInitHelper`
is deliberately not used -- its `setDelegate` replaces the singleton for the whole
process, which would quietly turn `ConversionWorkerTest` into a synchronous test double
depending on class order.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,73 @@
|
||||
package org.libremediaconverter.work
|
||||
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* What has to survive a restart, and what a malformed tag does.
|
||||
*
|
||||
* The values come from a picker and go into WorkManager's database, so the encoder and the
|
||||
* decoder are the two halves of one round trip and are tested as one. The lenient reads
|
||||
* matter as much as the round trip: work enqueued by an older version of the app carries none
|
||||
* of these tags, and it is exactly the work most likely to still be queued the first time
|
||||
* this code runs.
|
||||
*/
|
||||
class JobTagsTest {
|
||||
|
||||
@Test
|
||||
fun `a display name survives the round trip`() {
|
||||
val tags = setOf("org.libremediaconverter.work.ConversionWorker", JobTags.displayName("holiday.mp4"))
|
||||
assertEquals("holiday.mp4", JobTags.displayNameOf(tags))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a display name that looks like another tag is still read back whole`() {
|
||||
// Tags are matched by prefix over the whole string, so a file named after one of the
|
||||
// other prefixes cannot be mistaken for it.
|
||||
val name = "lmc.size-bytes:9"
|
||||
val tags = setOf(JobTags.displayName(name), JobTags.sizeBytes(4096))
|
||||
assertEquals(name, JobTags.displayNameOf(tags))
|
||||
assertEquals(4096L, JobTags.sizeBytesOf(tags))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a display name with spaces, colons and unicode is carried verbatim`() {
|
||||
val name = "холидей: clip 2 — final.mkv"
|
||||
assertEquals(name, JobTags.displayNameOf(setOf(JobTags.displayName(name))))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a size survives the round trip`() {
|
||||
assertEquals(9_000_000_000L, JobTags.sizeBytesOf(setOf(JobTags.sizeBytes(9_000_000_000L))))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an input count survives the round trip`() {
|
||||
assertEquals(7, JobTags.inputCountOf(setOf(JobTags.inputCount(7))))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a job with no tags of ours reads back as nothing known`() {
|
||||
// Work enqueued before this app version. The reattachment falls back rather than
|
||||
// skipping the job, because the job is still the user's file.
|
||||
val tags = setOf("org.libremediaconverter.work.ConversionWorker")
|
||||
assertNull(JobTags.displayNameOf(tags))
|
||||
assertNull(JobTags.sizeBytesOf(tags))
|
||||
assertNull(JobTags.inputCountOf(tags))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a size that is not a number reads as unknown rather than throwing`() {
|
||||
assertNull(JobTags.sizeBytesOf(setOf("lmc.size-bytes:huge")))
|
||||
assertNull(JobTags.inputCountOf(setOf("lmc.input-count:")))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the three tags do not read each other`() {
|
||||
val tags = setOf(JobTags.displayName("clip.mp4"), JobTags.sizeBytes(12), JobTags.inputCount(3))
|
||||
assertEquals("clip.mp4", JobTags.displayNameOf(tags))
|
||||
assertEquals(12L, JobTags.sizeBytesOf(tags))
|
||||
assertEquals(3, JobTags.inputCountOf(tags))
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,220 @@
|
||||
package org.libremediaconverter.work
|
||||
|
||||
import androidx.work.WorkInfo
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Test
|
||||
import java.util.UUID
|
||||
|
||||
/**
|
||||
* The rule for picking up a job the ViewModel did not start.
|
||||
*
|
||||
* Isolated from the ViewModel precisely so it can be tested: reaching this code for real means
|
||||
* the process being reclaimed while a job — or a result nobody saved — is still around, which
|
||||
* needs a device and an `am kill`. Extracting the choice means the rule is verified even though
|
||||
* the situation that calls for it cannot be reproduced on the JVM.
|
||||
*/
|
||||
class ReattachmentTest {
|
||||
|
||||
@Test
|
||||
fun `nothing to reattach to when there is no work at all`() {
|
||||
assertNull(Reattachment.choose(emptyList()))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a cancelled job is never reattached to`() {
|
||||
// The user already said no. Bringing it back would undo that.
|
||||
val cancelled = job(state = WorkInfo.State.CANCELLED, outputPath = "/cache/out.mp4", outputExists = true)
|
||||
assertNull(Reattachment.choose(listOf(cancelled)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a failed job is not reattached to`() {
|
||||
// Nothing marks a failure as seen, so it would reappear on every launch. Failures left
|
||||
// by an interrupted worker are ordinary: a restart's setForeground can be refused.
|
||||
assertNull(Reattachment.choose(listOf(job(state = WorkInfo.State.FAILED))))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a finished result still on disk is offered`() {
|
||||
val result = job(state = WorkInfo.State.SUCCEEDED, outputPath = "/cache/out.mp4", outputExists = true)
|
||||
assertEquals(Reattachment.Certain(result), Reattachment.choose(listOf(result)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a finished result whose staged file is gone is not offered`() {
|
||||
// Saved already, or the OS reclaimed the cache. A Save button here would fail on tap.
|
||||
val vanished = job(state = WorkInfo.State.SUCCEEDED, outputPath = "/cache/out.mp4", outputExists = false)
|
||||
assertNull(Reattachment.choose(listOf(vanished)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a job that reported success without a path is not offered`() {
|
||||
val pathless = job(state = WorkInfo.State.SUCCEEDED, outputPath = null, outputExists = false)
|
||||
assertNull(Reattachment.choose(listOf(pathless)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a running job is preferred to a finished result`() {
|
||||
// A running job holds a foreground notification. Someone opening the app while that
|
||||
// notification is in the shade is looking for that conversion.
|
||||
val result = job(state = WorkInfo.State.SUCCEEDED, outputPath = "/cache/out.mp4", outputExists = true)
|
||||
val running = job(state = WorkInfo.State.RUNNING)
|
||||
assertEquals(Reattachment.Certain(running), Reattachment.choose(listOf(result, running)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a running job is preferred to a queued one`() {
|
||||
val queued = job(state = WorkInfo.State.ENQUEUED)
|
||||
val running = job(state = WorkInfo.State.RUNNING)
|
||||
assertEquals(Reattachment.Certain(running), Reattachment.choose(listOf(queued, running)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a job waiting to retry is preferred to one that has never run`() {
|
||||
// It has already done part of the work — most likely it exhausted the foreground
|
||||
// budget mid-conversion — so it is the one closer to producing a file.
|
||||
val fresh = job(state = WorkInfo.State.ENQUEUED, runAttemptCount = 0)
|
||||
val retrying = job(state = WorkInfo.State.ENQUEUED, runAttemptCount = 1)
|
||||
assertEquals(Reattachment.Certain(retrying), Reattachment.choose(listOf(fresh, retrying)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a queued job is preferred to a finished result`() {
|
||||
val result = job(state = WorkInfo.State.SUCCEEDED, outputPath = "/cache/out.mp4", outputExists = true)
|
||||
val queued = job(state = WorkInfo.State.ENQUEUED)
|
||||
assertEquals(Reattachment.Certain(queued), Reattachment.choose(listOf(result, queued)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `blocked work counts as queued rather than being ignored`() {
|
||||
val blocked = job(state = WorkInfo.State.BLOCKED)
|
||||
assertEquals(Reattachment.Certain(blocked), Reattachment.choose(listOf(blocked)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the newer of two results is the one offered`() {
|
||||
// Losing this tie is not the same as waiting for the next launch: the query has no
|
||||
// ORDER BY, so an arbitrary winner would win every launch and the other result would
|
||||
// stay unreachable for as long as its file existed.
|
||||
val older = finishedResult(STAGED, tags = emptySet(), modifiedAt = 1_000L)
|
||||
val newer = finishedResult("/cache/beach_converted.mp4", tags = emptySet(), modifiedAt = 2_000L)
|
||||
|
||||
assertEquals(Reattachment.Certain(newer), Reattachment.choose(listOf(older, newer)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a newer result still does not outrank live work`() {
|
||||
// Rank first, time second. A running job holds the notification the user is following.
|
||||
val running = job(state = WorkInfo.State.RUNNING)
|
||||
val newer = finishedResult(STAGED, tags = emptySet(), modifiedAt = Long.MAX_VALUE)
|
||||
|
||||
assertEquals(Reattachment.Certain(running), Reattachment.choose(listOf(newer, running)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `two live jobs, which have written nothing to compare, resolve to the query's order`() {
|
||||
val first = job(state = WorkInfo.State.RUNNING)
|
||||
val second = job(state = WorkInfo.State.RUNNING)
|
||||
assertEquals(Reattachment.Certain(first), Reattachment.choose(listOf(first, second)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a result is still found when everything else is unusable`() {
|
||||
val result = job(state = WorkInfo.State.SUCCEEDED, outputPath = "/cache/out.mp4", outputExists = true)
|
||||
val jobs = listOf(
|
||||
job(state = WorkInfo.State.CANCELLED),
|
||||
job(state = WorkInfo.State.FAILED),
|
||||
job(state = WorkInfo.State.SUCCEEDED, outputPath = "/cache/gone.mp4", outputExists = false),
|
||||
result,
|
||||
)
|
||||
assertEquals(Reattachment.Certain(result), Reattachment.choose(jobs))
|
||||
}
|
||||
|
||||
// --- when two jobs claim the same staged file ---------------------------------------------
|
||||
|
||||
@Test
|
||||
fun `two results naming the same file are still offered, but not attributed`() {
|
||||
// Straight off a device: two SUCCEEDED jobs whose output path was the same
|
||||
// input_converted.mp4, with one file on disk. The file is the user's either way; which
|
||||
// job wrote it is not knowable, so the caller is told not to describe it.
|
||||
val first = finishedResult(STAGED, tags = setOf(JobTags.displayName("holiday.mp4")))
|
||||
val second = finishedResult(STAGED, tags = setOf(JobTags.displayName("holiday.mkv")))
|
||||
|
||||
assertEquals(Reattachment.Ambiguous(first), Reattachment.choose(listOf(first, second)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `aliases that describe the same input are attributed after all`() {
|
||||
// The ordinary way to end up with two: convert the same file twice. Nothing turns on
|
||||
// which of them wrote the file, so the label is safe.
|
||||
val tags = setOf(JobTags.displayName("holiday.mp4"), JobTags.sizeBytes(4_096))
|
||||
val first = finishedResult(STAGED, tags = tags)
|
||||
val second = finishedResult(STAGED, tags = tags)
|
||||
|
||||
assertEquals(Reattachment.Certain(first), Reattachment.choose(listOf(first, second)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `results naming different files do not make each other ambiguous`() {
|
||||
val first = finishedResult(STAGED, tags = setOf(JobTags.displayName("holiday.mp4")))
|
||||
val second = finishedResult("/cache/beach_converted.mp4", tags = setOf(JobTags.displayName("beach.mp4")))
|
||||
|
||||
assertEquals(Reattachment.Certain(first), Reattachment.choose(listOf(first, second)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a job with no file yet is not aliased by every other job without one`() {
|
||||
// Guards the obvious mistake: live jobs all carry a null output path, and grouping on
|
||||
// that would make each of them ambiguous with all the others.
|
||||
val running = job(state = WorkInfo.State.RUNNING, tags = setOf(JobTags.displayName("holiday.mp4")))
|
||||
val queued = job(state = WorkInfo.State.ENQUEUED, tags = setOf(JobTags.displayName("beach.mp4")))
|
||||
|
||||
assertEquals(Reattachment.Certain(running), Reattachment.choose(listOf(running, queued)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an unusable alias does not make a result ambiguous`() {
|
||||
// A cancelled or failed job deletes its staged file on the way out, so it never wrote
|
||||
// what is on disk now and says nothing about who did.
|
||||
val result = finishedResult(STAGED, tags = setOf(JobTags.displayName("holiday.mp4")))
|
||||
val abandoned = job(
|
||||
state = WorkInfo.State.CANCELLED,
|
||||
outputPath = STAGED,
|
||||
outputExists = true,
|
||||
tags = setOf(JobTags.displayName("something else.mp4")),
|
||||
)
|
||||
|
||||
assertEquals(Reattachment.Certain(result), Reattachment.choose(listOf(result, abandoned)))
|
||||
}
|
||||
|
||||
private fun finishedResult(path: String, tags: Set<String>, modifiedAt: Long = 0L) = job(
|
||||
state = WorkInfo.State.SUCCEEDED,
|
||||
outputPath = path,
|
||||
outputExists = true,
|
||||
tags = tags,
|
||||
modifiedAt = modifiedAt,
|
||||
)
|
||||
|
||||
private fun job(
|
||||
state: WorkInfo.State,
|
||||
runAttemptCount: Int = 0,
|
||||
outputPath: String? = null,
|
||||
outputExists: Boolean = false,
|
||||
tags: Set<String> = emptySet(),
|
||||
modifiedAt: Long = 0L,
|
||||
) = JobSnapshot(
|
||||
id = UUID.randomUUID(),
|
||||
state = state,
|
||||
runAttemptCount = runAttemptCount,
|
||||
outputPath = outputPath,
|
||||
outputExists = outputExists,
|
||||
outputModifiedAt = modifiedAt,
|
||||
tags = tags,
|
||||
)
|
||||
|
||||
private companion object {
|
||||
/** One staging path, because the interesting cases are the ones that share it. */
|
||||
const val STAGED = "/cache/holiday_converted.mp4"
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user