The roster guard is a decision that can be run, not grepped
Same as the iPhone. The rule lives in shouldAdoptRoster and the test asks it rather than searching Session.kt for a line: empty never replaces held, empty onto empty is allowed, anything non-empty always lands. A guard that refused everything would have passed the old source-reading test and broken the app. The ordering test stays, since where the guard sits relative to the write is a real property: after it, the empty list is saved on the way past and the fallback reads it back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
73c7b7baef
commit
dafb4f8adb
@@ -2110,7 +2110,9 @@ class Session(
|
||||
// it would erase the stored roster on the first poll after an
|
||||
// operator threw that switch, and on any transient fault that
|
||||
// answered empty. Nothing on the server can empty a roster now.
|
||||
if (fetched.isEmpty() && items.length() > 0) return stored()
|
||||
if (!shouldAdoptRoster(incoming = fetched.size, held = items.length())) {
|
||||
return stored()
|
||||
}
|
||||
val mirror = org.json.JSONArray()
|
||||
fetched.forEach { b ->
|
||||
mirror.put(
|
||||
@@ -2151,6 +2153,35 @@ class Session(
|
||||
* since the last pass. Both are best effort: a relay that refuses either
|
||||
* leaves presence exactly where the roster put it.
|
||||
*/
|
||||
/**
|
||||
* Whether a roster arriving from the relay should replace the one stored.
|
||||
*
|
||||
* Its own function so the decision can be exercised directly. The rule is
|
||||
* one line and the consequence of getting it wrong is somebody's contacts,
|
||||
* which is not a thing to leave provable only by reading the source.
|
||||
*
|
||||
* Empty NEVER replaces something: a relay that has stopped keeping the
|
||||
* contact graph (§8.9c) answers with an empty list, and so does a
|
||||
* transient fault. Empty onto empty is allowed, or a new account could
|
||||
* never take its first contacts from the server.
|
||||
*/
|
||||
internal fun shouldAdoptRoster(
|
||||
incoming: Int,
|
||||
held: Int,
|
||||
): Boolean = shouldAdoptRosterForTest(incoming, held)
|
||||
|
||||
companion object {
|
||||
/**
|
||||
* The rule, reachable without building a Session so a test can ask it
|
||||
* directly rather than grep for it.
|
||||
*/
|
||||
@JvmStatic
|
||||
internal fun shouldAdoptRosterForTest(
|
||||
incoming: Int,
|
||||
held: Int,
|
||||
): Boolean = incoming > 0 || held == 0
|
||||
}
|
||||
|
||||
private fun presenceFromTokens(): Map<Long, String> {
|
||||
val held = conversations.presenceTokensHeld()
|
||||
// Compared against what the relay last ACCEPTED, not driven by a flag
|
||||
|
||||
@@ -2,6 +2,7 @@ package io.flamenet.messenger.app
|
||||
|
||||
import java.io.File
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertFalse
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
/**
|
||||
@@ -20,12 +21,34 @@ class AnEmptyRosterNeverErasesOneTest {
|
||||
it.readText()
|
||||
}
|
||||
|
||||
/**
|
||||
* The decision itself, run rather than read. Testing this by grepping the
|
||||
* source was not good enough for something whose failure mode is
|
||||
* somebody's contact list.
|
||||
*/
|
||||
@Test
|
||||
fun `an empty answer falls back to what is stored`() {
|
||||
assertTrue(
|
||||
session.contains("if (fetched.isEmpty() && items.length() > 0) return stored()"),
|
||||
"an empty server answer would be mirrored over the stored roster",
|
||||
)
|
||||
fun `an empty list never replaces a roster that is held`() {
|
||||
assertFalse(Session.shouldAdoptRosterForTest(incoming = 0, held = 7))
|
||||
assertFalse(Session.shouldAdoptRosterForTest(incoming = 0, held = 1))
|
||||
}
|
||||
|
||||
/**
|
||||
* Empty onto empty is allowed, or a new account could never take its first
|
||||
* contacts from the server.
|
||||
*/
|
||||
@Test
|
||||
fun `an empty store still accepts an empty answer`() {
|
||||
assertTrue(Session.shouldAdoptRosterForTest(incoming = 0, held = 0))
|
||||
}
|
||||
|
||||
/**
|
||||
* And a real list still lands. A guard that refused everything would pass
|
||||
* the test above and break the app.
|
||||
*/
|
||||
@Test
|
||||
fun `a list with anything in it is always adopted`() {
|
||||
assertTrue(Session.shouldAdoptRosterForTest(incoming = 1, held = 0))
|
||||
assertTrue(Session.shouldAdoptRosterForTest(incoming = 1, held = 9))
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -34,20 +57,8 @@ class AnEmptyRosterNeverErasesOneTest {
|
||||
*/
|
||||
@Test
|
||||
fun `the guard comes before the write`() {
|
||||
val guard = session.indexOf("if (fetched.isEmpty() && items.length() > 0)")
|
||||
val guard = session.indexOf("shouldAdoptRoster(incoming = fetched.size")
|
||||
val write = session.indexOf("vault.saveRoster(mirror, offServer = false)")
|
||||
assertTrue(guard in 0..<write, "guard at $guard, write at $write")
|
||||
}
|
||||
|
||||
/**
|
||||
* An empty list is still adoptable onto an empty store, or a new account
|
||||
* could never take its first contact from the server.
|
||||
*/
|
||||
@Test
|
||||
fun `an empty store still accepts an empty answer`() {
|
||||
assertTrue(
|
||||
session.contains("items.length() > 0"),
|
||||
"the guard must only fire when something is actually held",
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user