summaryrefslogtreecommitdiff
path: root/android/app/src/test/java/org/terst/doot/widget
diff options
context:
space:
mode:
authorPeter Stone <thepeterstone@gmail.com>2026-08-05 00:36:18 +0000
committerPeter Stone <thepeterstone@gmail.com>2026-08-05 00:36:18 +0000
commit7b9eb14db9b953b3c5882b3b85c819d9baa2dd2c (patch)
treea5d4fa8959186d8f34da6dede582d4b0c58ce154 /android/app/src/test/java/org/terst/doot/widget
parent0400e59e5d4a0140e98af01ad1acbc83d3f3f0a7 (diff)
Widget: fix "complete works once, then stops" race between overlapping completions
Root cause traced from server logs, not guessed: every completion request was succeeding server-side (100% 200s, including a 5-tap burst spanning different tasks), and each successful completion's response payload was correctly shrinking. So the failure wasn't dispatch or network -- it was that a successful completion could still get silently undone client-side. fetchAndPersist does an unconditional full overwrite of the cached item list on every successful GET. CompleteWorker/DeferWorker run one instance per task id with no ordering guarantee between different ids' workers (different unique work names, no KEEP protection across them -- that protection only ever covered same-task double-taps). So: tapping complete on task A starts a GET that's still in flight; tapping complete on task B before A's GET returns optimistically removes B locally; A's slower GET response, captured before B's completion landed, then overwrites the cache and silently resurrects B. Fix: track locally-optimistic removals with a timestamp (PendingRemovals.kt) and filter them out of every fetchAndPersist write for a bounded TTL (2 min), regardless of which worker's fetch is doing the writing. The TTL means a completion that never actually confirms (permanent network failure) still self-heals via the next periodic refresh, matching an existing self-healing property already relied on elsewhere in this codebase, instead of hiding the task forever. Added PendingRemovalsTest.kt (pure-function unit tests, no Android runtime needed) covering the exact race scenario plus TTL expiry and edge cases. Verified the tests actually catch a regression by deliberately reverting the fix to a no-op against a real backup, confirming 3 tests failed with the exact expected assertion, then restoring and confirming green again. Built, tested, and published as doot-widget.apk.
Diffstat (limited to 'android/app/src/test/java/org/terst/doot/widget')
-rw-r--r--android/app/src/test/java/org/terst/doot/widget/PendingRemovalsTest.kt107
1 files changed, 107 insertions, 0 deletions
diff --git a/android/app/src/test/java/org/terst/doot/widget/PendingRemovalsTest.kt b/android/app/src/test/java/org/terst/doot/widget/PendingRemovalsTest.kt
new file mode 100644
index 0000000..415bfd1
--- /dev/null
+++ b/android/app/src/test/java/org/terst/doot/widget/PendingRemovalsTest.kt
@@ -0,0 +1,107 @@
+package org.terst.doot.widget
+
+import org.junit.Assert.*
+import org.junit.Test
+import org.terst.doot.widget.data.PENDING_REMOVAL_TTL_MS
+import org.terst.doot.widget.data.WidgetItem
+import org.terst.doot.widget.data.decodePendingRemovals
+import org.terst.doot.widget.data.encodePendingRemovals
+import org.terst.doot.widget.data.excludingUnexpiredPendingRemovals
+import org.terst.doot.widget.data.pendingKey
+import org.terst.doot.widget.data.pruneExpired
+
+class PendingRemovalsTest {
+
+ private fun taskItem(id: String, source: String = "doot") =
+ WidgetItem(id = id, title = "Task $id", source = source, type = "task")
+
+ @Test
+ fun `excludingUnexpiredPendingRemovals filters out a pending item`() {
+ val items = listOf(taskItem("a"), taskItem("b"))
+ val now = 1_000_000L
+ val pending = mapOf(pendingKey("b", "doot") to now)
+
+ val result = items.excludingUnexpiredPendingRemovals(pending, now)
+
+ assertEquals(listOf("a"), result.map { it.id })
+ }
+
+ @Test
+ fun `excludingUnexpiredPendingRemovals is a no-op with an empty pending map`() {
+ val items = listOf(taskItem("a"), taskItem("b"))
+ assertEquals(items, items.excludingUnexpiredPendingRemovals(emptyMap(), 1_000_000L))
+ }
+
+ // This is the actual regression: a slower, earlier-started fetch's response still
+ // includes an item that a faster, later-started completion has since removed. Without
+ // pending-removal protection, persisting that stale snapshot resurrects the item --
+ // this is what made "check to complete" look like it stopped working after the first
+ // tap (2026-08-05), even though every completion request was independently succeeding
+ // server-side.
+ @Test
+ fun `a stale fetch snapshot captured before a completion cannot resurrect it`() {
+ val staleSnapshotStillContainingB = listOf(taskItem("a"), taskItem("b"))
+ val completedAt = 1_000_000L
+ val pendingAfterCompletingB = mapOf(pendingKey("b", "doot") to completedAt)
+
+ // The stale fetch's write happens *after* b was completed, wall-clock, even though
+ // its data was captured before -- exactly the race: slower fetch, later write.
+ val persisted = staleSnapshotStillContainingB
+ .excludingUnexpiredPendingRemovals(pendingAfterCompletingB, completedAt + 500)
+
+ assertEquals(listOf("a"), persisted.map { it.id })
+ }
+
+ @Test
+ fun `a pending removal expires and stops protecting after the TTL`() {
+ val items = listOf(taskItem("a"), taskItem("b"))
+ val removedAt = 1_000_000L
+ val pending = mapOf(pendingKey("b", "doot") to removedAt)
+
+ // Just before expiry: still protected.
+ val stillProtected = items.excludingUnexpiredPendingRemovals(pending, removedAt + PENDING_REMOVAL_TTL_MS - 1)
+ assertEquals(listOf("a"), stillProtected.map { it.id })
+
+ // At/after expiry: self-healing kicks in -- a permanently-failed completion must
+ // not hide its task forever, matching the existing periodic-refresh self-healing
+ // property elsewhere in this codebase.
+ val expired = items.excludingUnexpiredPendingRemovals(pending, removedAt + PENDING_REMOVAL_TTL_MS)
+ assertEquals(listOf("a", "b"), expired.map { it.id })
+ }
+
+ @Test
+ fun `different source with the same id is not confused for the pending item`() {
+ val items = listOf(taskItem("shared-id", source = "trello"))
+ val pending = mapOf(pendingKey("shared-id", "doot") to 1_000_000L)
+
+ val result = items.excludingUnexpiredPendingRemovals(pending, 1_000_000L)
+
+ assertEquals(listOf("shared-id"), result.map { it.id })
+ }
+
+ @Test
+ fun `pruneExpired drops old entries and keeps fresh ones`() {
+ val now = 1_000_000L
+ val pending = mapOf(
+ "fresh" to now,
+ "stale" to now - PENDING_REMOVAL_TTL_MS
+ )
+
+ val pruned = pending.pruneExpired(now)
+
+ assertEquals(setOf("fresh"), pruned.keys)
+ }
+
+ @Test
+ fun `encode and decode round-trip`() {
+ val pending = mapOf(pendingKey("a", "doot") to 42L)
+ val decoded = decodePendingRemovals(encodePendingRemovals(pending))
+ assertEquals(pending, decoded)
+ }
+
+ @Test
+ fun `decodePendingRemovals treats null and malformed input as empty`() {
+ assertEquals(emptyMap<String, Long>(), decodePendingRemovals(null))
+ assertEquals(emptyMap<String, Long>(), decodePendingRemovals("not json"))
+ }
+}