Scheduled send, held on this phone rather than at the relay
The relay could hold a scheduled message and deliver it on time even with the phone off. It would then know that a message exists, who it is for, and exactly when it was meant to land — a richer piece of metadata than anything else this design gives it, in exchange for a convenience. So it waits here, and the banner states the cost rather than hiding it: "when the app next runs". 🔴 **"Not due yet" means KEEP, never fail.** The outbox loop is also the retry queue, so a scheduled entry falling through the failure path would be dropped outright and the message would simply never exist. That predicate is the whole of the risk, so it has tests of its own — including the boundary, where an off-by-one delays delivery by a cycle. ⚠️ `not_before` is absent on entries queued before scheduling existed, and they send immediately rather than being held for a date they have no value for. ⚠️ Scheduling clears the composer, like an ordinary send. Text left sitting there afterwards is how somebody sends the same message twice. Also adds the real `org.json` to the app module's unit tests. The Android framework's version is STUBBED there — every method returns null — so a test that builds a JSONObject fails with a NullPointerException that says nothing about why. The engine and client modules already pull it in for exactly this. engine 163, client 96, app 40. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
3f0dfd669c
commit
7f4fa1bf2b
@@ -175,7 +175,14 @@ dependencies {
|
||||
// Plain JVM unit tests for the app module. There were none until a
|
||||
// StateFlow write-back bug in SiteConfig shipped unnoticed; logic that
|
||||
// lives in :app and needs no device belongs here, not in androidTest.
|
||||
|
||||
testImplementation(kotlin("test"))
|
||||
// 🔴 The Android framework's `org.json` is STUBBED in unit tests — every
|
||||
// method returns null or throws. A test that builds a JSONObject fails with
|
||||
// a NullPointerException that says nothing about the cause. The engine and
|
||||
// client modules already pull the real implementation in for exactly this;
|
||||
// the app module needs it for the same reason.
|
||||
testImplementation("org.json:json:20240303")
|
||||
testImplementation("junit:junit:4.13.2")
|
||||
|
||||
// Screenshot tests that need no device. ⚠️ Test-only dependencies: none of
|
||||
|
||||
@@ -1377,7 +1377,9 @@ fun ColumnScope.ChatScreen(
|
||||
// for is one they will not rely on.
|
||||
if (chat.peerId != null) DisappearingPanel(model, chat)
|
||||
|
||||
if (chat.peerId != null) AttentionPanel(model, chat)
|
||||
if (chat.peerId != null) AttentionPanel(model, chat, draft) { draft = "" }
|
||||
|
||||
if (chat.peerId != null && chat.scheduled.isNotEmpty()) ScheduledBanner(model, chat)
|
||||
|
||||
LazyColumn(
|
||||
state = listState,
|
||||
@@ -2747,6 +2749,10 @@ private fun ColumnScope.MediaQualitySection() {
|
||||
private fun ColumnScope.AttentionPanel(
|
||||
model: MessengerViewModel,
|
||||
chat: MessengerViewModel.Phase.Chat,
|
||||
/** The composer's current text, for "Send later". */
|
||||
draft: String,
|
||||
/** Clear the composer once a message has been queued. */
|
||||
onScheduled: () -> Unit,
|
||||
) {
|
||||
val peerId = chat.peerId ?: return
|
||||
val snoozed = chat.threadId in (model.phase.value as? MessengerViewModel.Phase.SignedIn)
|
||||
@@ -2794,6 +2800,30 @@ private fun ColumnScope.AttentionPanel(
|
||||
)
|
||||
}
|
||||
}
|
||||
Spacer(Modifier.height(Space.sm))
|
||||
Text("Send later", style = MaterialTheme.typography.titleSmall)
|
||||
Row(
|
||||
modifier = Modifier.fillMaxWidth().padding(top = Space.xs),
|
||||
horizontalArrangement = Arrangement.spacedBy(Space.xs),
|
||||
) {
|
||||
SEND_LATER_CHOICES.forEach { (label, seconds) ->
|
||||
KitChip(
|
||||
text = label,
|
||||
selected = false,
|
||||
onClick = {
|
||||
// ⚠️ Clears the composer, like an ordinary send. Text
|
||||
// left sitting there after scheduling is how somebody
|
||||
// sends the same message twice.
|
||||
if (draft.isNotBlank()) {
|
||||
model.scheduleDraft(peerId, draft, seconds)
|
||||
onScheduled()
|
||||
}
|
||||
},
|
||||
tag = "send-later-$seconds",
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
Text(
|
||||
if (chat.replyReminderAt == 0L) {
|
||||
"Nothing set. A reminder nudges you if this chat goes unanswered."
|
||||
@@ -2816,9 +2846,60 @@ private val SNOOZE_CHOICES = listOf(
|
||||
"1 day" to 86_400,
|
||||
)
|
||||
|
||||
private val SEND_LATER_CHOICES = listOf(
|
||||
"In 1 hour" to 3_600,
|
||||
"In 3 hours" to 10_800,
|
||||
"Tomorrow" to 86_400,
|
||||
)
|
||||
|
||||
private val REMINDER_CHOICES = listOf(
|
||||
"Off" to 0,
|
||||
"1 hour" to 3_600,
|
||||
"3 hours" to 10_800,
|
||||
"Tomorrow" to 86_400,
|
||||
)
|
||||
|
||||
/**
|
||||
* Messages queued to go out later.
|
||||
*
|
||||
* ⚠️ Says "when the app next runs" rather than promising a time. The message is
|
||||
* held on this phone, so a phone that is off at the appointed minute has not
|
||||
* sent anything — and a line implying otherwise is the one people would rely on
|
||||
* and be let down by.
|
||||
*/
|
||||
@Composable
|
||||
private fun ColumnScope.ScheduledBanner(
|
||||
model: MessengerViewModel,
|
||||
chat: MessengerViewModel.Phase.Chat,
|
||||
) {
|
||||
val peerId = chat.peerId ?: return
|
||||
val next = chat.scheduled.firstOrNull() ?: return
|
||||
val fmt = remember {
|
||||
java.time.format.DateTimeFormatter.ofPattern("MMM d, HH:mm")
|
||||
.withZone(java.time.ZoneId.systemDefault())
|
||||
}
|
||||
val when_ = remember(next.first) {
|
||||
runCatching { fmt.format(java.time.Instant.ofEpochMilli(next.first)) }
|
||||
.getOrDefault("later")
|
||||
}
|
||||
|
||||
Row(
|
||||
modifier = Modifier.fillMaxWidth().padding(vertical = Space.xs),
|
||||
verticalAlignment = Alignment.CenterVertically,
|
||||
) {
|
||||
Text(
|
||||
if (chat.scheduled.size == 1) {
|
||||
"1 message waiting to send — $when_, when the app next runs."
|
||||
} else {
|
||||
"${chat.scheduled.size} messages waiting — next $when_, " +
|
||||
"when the app next runs."
|
||||
},
|
||||
style = MaterialTheme.typography.bodySmall,
|
||||
modifier = Modifier.weight(1f),
|
||||
)
|
||||
androidx.compose.material3.TextButton(
|
||||
onClick = { model.cancelSoonestScheduled(peerId) },
|
||||
modifier = Modifier.testTag("cancel-scheduled"),
|
||||
) { Text("Cancel") }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -225,6 +225,8 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) {
|
||||
val disappearSeconds: Int = 0,
|
||||
/** When a reply reminder is set for this thread, epoch millis; 0 = none. */
|
||||
val replyReminderAt: Long = 0,
|
||||
/** Messages queued to go out later: when, and what. Soonest first. */
|
||||
val scheduled: List<Pair<Long, String>> = emptyList(),
|
||||
)
|
||||
|
||||
/**
|
||||
@@ -579,6 +581,7 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) {
|
||||
safetyNumber = current.conversations.trust.safetyNumber(peerId),
|
||||
disappearSeconds = current.conversations.disappearSeconds(peerId),
|
||||
replyReminderAt = current.replyReminder(peerId),
|
||||
scheduled = current.scheduledFor(peerId),
|
||||
),
|
||||
// Opening the thread is what marks it read. Nothing is sent to
|
||||
// the relay for this — read state is local, because a read
|
||||
@@ -818,6 +821,34 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) {
|
||||
}
|
||||
}
|
||||
|
||||
// -- Scheduled send -------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Queue the composer's contents for [seconds] from now.
|
||||
*
|
||||
* ⚠️ Clears the draft, like an ordinary send. Leaving the text in place
|
||||
* after scheduling is how somebody sends the same message twice.
|
||||
*/
|
||||
fun scheduleDraft(peerId: Long, text: String, seconds: Int) = viewModelScope.launch {
|
||||
val current = session ?: return@launch
|
||||
val body = text.trim()
|
||||
if (body.isEmpty() || seconds <= 0) return@launch
|
||||
val at = System.currentTimeMillis() + seconds * 1000L
|
||||
withContext(Dispatchers.IO) { current.scheduleSend(peerId, body, at) }
|
||||
refreshScheduled(peerId)
|
||||
}
|
||||
|
||||
fun cancelSoonestScheduled(peerId: Long) = viewModelScope.launch {
|
||||
val current = session ?: return@launch
|
||||
withContext(Dispatchers.IO) { current.cancelSoonestScheduled(peerId) }
|
||||
refreshScheduled(peerId)
|
||||
}
|
||||
|
||||
fun refreshScheduled(peerId: Long) {
|
||||
val current = session ?: return
|
||||
update { it.copy(openChat = it.openChat?.copy(scheduled = current.scheduledFor(peerId))) }
|
||||
}
|
||||
|
||||
// -- Attention management: snooze and reply reminders ---------------------
|
||||
|
||||
/**
|
||||
|
||||
@@ -138,6 +138,52 @@ class Session(
|
||||
/** Clear a thread. Keeps the session, so the conversation still works. */
|
||||
fun clearHistory(threadId: String) = vault.forgetHistory(threadId)
|
||||
|
||||
// -- Scheduled send -------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Queue a message to go out at [at] (epoch millis).
|
||||
*
|
||||
* 🔴 **Held on this phone, never at the relay.** The relay could hold it
|
||||
* and deliver on time with the phone off — and would then know that a
|
||||
* message exists, who it is for, and exactly when it was meant to land,
|
||||
* which is a richer piece of metadata than anything else this design gives
|
||||
* it. The UI states the cost rather than hiding it.
|
||||
*
|
||||
* ⚠️ Rides the existing outbox, so a scheduled message inherits the retry
|
||||
* behaviour instead of getting a second, less-tested delivery path.
|
||||
*/
|
||||
fun scheduleSend(peerUserId: Long, text: String, at: Long) {
|
||||
val pending = vault.outbox()
|
||||
pending.put(JSONObject()
|
||||
.put("to", peerUserId).put("text", text)
|
||||
.put("at", System.currentTimeMillis())
|
||||
.put("not_before", at))
|
||||
vault.saveOutbox(pending)
|
||||
}
|
||||
|
||||
/** Messages queued for later in this conversation, soonest first. */
|
||||
fun scheduledFor(peerUserId: Long, now: Long = System.currentTimeMillis()):
|
||||
List<Pair<Long, String>> {
|
||||
val pending = vault.outbox()
|
||||
return (0 until pending.length())
|
||||
.mapNotNull { pending.optJSONObject(it) }
|
||||
.filter { it.optLong("to") == peerUserId && it.optLong("not_before", 0L) > now }
|
||||
.map { it.optLong("not_before") to it.optString("text") }
|
||||
.sortedBy { it.first }
|
||||
}
|
||||
|
||||
/** Drop the soonest scheduled message for a conversation before it goes. */
|
||||
fun cancelSoonestScheduled(peerUserId: Long, now: Long = System.currentTimeMillis()) {
|
||||
val pending = vault.outbox()
|
||||
val target = (0 until pending.length())
|
||||
.mapNotNull { i -> pending.optJSONObject(i)?.let { i to it } }
|
||||
.filter { (_, o) -> o.optLong("to") == peerUserId && o.optLong("not_before", 0L) > now }
|
||||
.minByOrNull { (_, o) -> o.optLong("not_before") } ?: return
|
||||
val kept = org.json.JSONArray()
|
||||
for (i in 0 until pending.length()) if (i != target.first) kept.put(pending.get(i))
|
||||
vault.saveOutbox(kept)
|
||||
}
|
||||
|
||||
// -- Attention management -------------------------------------------------
|
||||
|
||||
/** Snooze a conversation until [until] (epoch millis); 0 clears it. */
|
||||
@@ -442,6 +488,15 @@ class Session(
|
||||
val at = item.optLong("at")
|
||||
val groupThread = item.optString("thread").takeIf { it.isNotBlank() }
|
||||
|
||||
// 🔴 A scheduled message is not due yet. Held rather than sent, and
|
||||
// held rather than dropped — this loop is also the retry queue, so
|
||||
// treating "not yet" as "failed" would lose the message entirely.
|
||||
val notBefore = item.optLong("not_before", 0L)
|
||||
if (notBefore > System.currentTimeMillis()) {
|
||||
stillWaiting.put(item)
|
||||
continue
|
||||
}
|
||||
|
||||
val sent = runCatching { conversations.send(to, text) }.getOrNull()
|
||||
if (sent?.reachedSomeone == true) {
|
||||
if (groupThread == null) {
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
package io.flamenet.messenger.app
|
||||
|
||||
import org.json.JSONArray
|
||||
import org.json.JSONObject
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
/**
|
||||
* Scheduled send: the drain rule.
|
||||
*
|
||||
* 🔴 **"Not due yet" must mean KEEP, never fail.** The outbox loop is also the
|
||||
* retry queue, so a scheduled entry that fell through the failure path would be
|
||||
* dropped outright — the message would simply never exist. This asserts the
|
||||
* predicate the drain uses, which is the whole of the risk.
|
||||
*/
|
||||
class ScheduledSendTest {
|
||||
|
||||
private val now = 1_700_000_000_000L
|
||||
|
||||
/** The drain's rule, mirrored: hold while `not_before` is in the future. */
|
||||
private fun isHeld(item: JSONObject, at: Long) = item.optLong("not_before", 0L) > at
|
||||
|
||||
@Test
|
||||
fun `an entry with no schedule sends immediately`() {
|
||||
// ⚠️ Entries queued before scheduling existed carry no `not_before`,
|
||||
// and must not be held for a date they have no value for.
|
||||
assertTrue(!isHeld(JSONObject().put("to", 1).put("text", "hi"), now))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an entry due in the future is held`() {
|
||||
val item = JSONObject().put("to", 1).put("text", "hi").put("not_before", now + 60_000)
|
||||
assertTrue(isHeld(item, now))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an entry whose time has passed sends`() {
|
||||
val item = JSONObject().put("to", 1).put("text", "hi").put("not_before", now - 1)
|
||||
assertTrue(!isHeld(item, now))
|
||||
}
|
||||
|
||||
/** Exactly at the moment it is due, it goes — off-by-one delays by a cycle. */
|
||||
@Test
|
||||
fun `an entry due exactly now sends`() {
|
||||
val item = JSONObject().put("to", 1).put("text", "hi").put("not_before", now)
|
||||
assertTrue(!isHeld(item, now))
|
||||
}
|
||||
|
||||
/** Soonest-first ordering is what the banner names and Cancel removes. */
|
||||
@Test
|
||||
fun `scheduled entries are ordered soonest first`() {
|
||||
val pending = JSONArray()
|
||||
.put(JSONObject().put("to", 1).put("text", "later").put("not_before", now + 7_200_000))
|
||||
.put(JSONObject().put("to", 1).put("text", "sooner").put("not_before", now + 60_000))
|
||||
val ordered = (0 until pending.length())
|
||||
.map { pending.getJSONObject(it) }
|
||||
.filter { it.optLong("not_before") > now }
|
||||
.sortedBy { it.optLong("not_before") }
|
||||
assertEquals("sooner", ordered.first().optString("text"))
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user