diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/UploadChaptersViewModel.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/UploadChaptersViewModel.kt index 5558c7c3..a32f752f 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/UploadChaptersViewModel.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/UploadChaptersViewModel.kt @@ -198,10 +198,20 @@ class UploadChaptersViewModel( onDone: (proposed: Int, failed: Int) -> Unit, ) { val loaded = _uiState.value as? UploadChaptersUIState.Loaded ?: return + + // A pass already running will call [onDone] itself when it finishes; + // firing it here as well would announce one pass twice. if (loaded.isProposing) return val queue = loaded.proposable.map { it.id } - if (queue.isEmpty()) return + if (queue.isEmpty()) { + // Nothing to do still has to be said. The FAB is disabled in this + // case so the screen never reaches it, but a callback that + // sometimes never fires is a trap for the next caller -- and it is + // what a test of a second tap found. + onDone(0, 0) + return + } updateLoaded { it.copy(isProposing = true) } diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/ChapterUploadJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/ChapterUploadJvmTest.kt new file mode 100644 index 00000000..50a44b3e --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/ChapterUploadJvmTest.kt @@ -0,0 +1,433 @@ +package press.mantra.compose.managers + +import androidx.room3.Room +import com.vitorpamplona.quartz.nip01Core.core.Event +import fr.acinq.bitcoin.ByteVector32 +import fr.acinq.bitcoin.PrivateKey +import fr.acinq.bitcoin.crypto.frost.Frost +import fr.acinq.bitcoin.crypto.frost.KeyMaterial +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull +import kotlin.test.assertTrue +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.setMain +import kotlinx.coroutines.withTimeout +import press.mantra.compose.database.MantraDatabase +import press.mantra.compose.database.builder.getRoomDatabase +import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.DkgParticipantMessage +import press.mantra.compose.database.model.DkgSession +import press.mantra.compose.database.model.MantraArtifact +import press.mantra.compose.database.model.MantraArtifactVersion +import press.mantra.compose.database.model.MantraDialect +import press.mantra.compose.database.model.MantraTranslationArtifactVersion +import press.mantra.compose.database.model.NostrEvent +import press.mantra.compose.database.model.Profile +import press.mantra.compose.database.model.types.DkgRitualStage +import press.mantra.compose.database.repository.DatabaseChatRepository +import press.mantra.compose.database.repository.DatabaseFrostSigningRepository +import press.mantra.compose.database.repository.DatabaseMantraRepository +import press.mantra.compose.extensions.toHex +import press.mantra.compose.files.PickedFile +import press.mantra.compose.nostr.dkg.DkgRitualEvents +import press.mantra.compose.nostr.nip30303.ChapterEvent +import press.mantra.compose.nostr.nip30303.ChunkEvent +import press.mantra.compose.nostr.nip30303.TranslationChapterEvent +import press.mantra.compose.text.Markdown +import press.mantra.compose.ui.view.model.UploadChaptersViewModel +import press.mantra.compose.ui.view.state.UploadChaptersUIState +import press.mantra.compose.ui.view.state.UploadProblem + +/** + * An upload, end to end, over a real database. + * + * `UploadReviewTest` covers what a file is offered as, which is pure. This is + * the orchestration: how many sessions a group is asked for, what each one + * signs, what index each chapter takes, and -- the part with no second chance -- + * what is left behind when a pass only half works. + * + * Nothing can retract a proposal, so partial success is the expected outcome + * rather than an error, and the only thing that makes it recoverable is the + * screen keeping an honest list of what went and what did not. + */ +class ChapterUploadJvmTest { + private val participants = 3 + private val threshold = 2 + + private val keyMaterial: KeyMaterial = Frost.trustedDealerKeygen( + thresholdSecretKey = PrivateKey( + ByteVector32("1c0ffee0000000000000000000000000000000000000000000000000000000a1") + ), + nParticipants = participants, + threshold = threshold, + ) + + private val thresholdPublicKey = keyMaterial.thresholdPublicKey.value.toHex() + private val roomId = SharedKeyDerivation.marmotGroupId(thresholdPublicKey) + + private val ceremonyId = "ceremony".padEnd(64, '0') + private val members = listOf("a", "b", "c").map { it.repeat(64) } + private val hostKeys = listOf("2a", "2b", "2c").map { it.padEnd(66, '0') } + + private val proposer = members.first() + private val dialectId = "d".repeat(64) + private val artifactId = "a1".padEnd(64, '0') + private val artifactVersionId = "b".repeat(64) + + private lateinit var db: MantraDatabase + private lateinit var viewModel: UploadChaptersViewModel + + @BeforeTest + fun setUpMain() { + // viewModelScope dispatches on Dispatchers.Main, which a jvm test has + // none of. Unconfined rather than a test scheduler: the view model does + // its work on Dispatchers.IO and uses Main only to hand a result back, + // so there is nothing here to advance -- the test waits on the callback. + Dispatchers.setMain(Dispatchers.Unconfined) + } + + @AfterTest + fun tearDown() { + Dispatchers.resetMain() + if (::db.isInitialized) db.close() + } + + /** Opens a device, with the group's shared key present only if [withKey]. */ + private suspend fun openDevice(withKey: Boolean = true, translations: Int = 0) { + db = getRoomDatabase(Room.inMemoryDatabaseBuilder()) + val scope = CoroutineScope(Dispatchers.Default + SupervisorJob()) + + val nostrEventId = "e$proposer".take(64) + db.nostrEventDao().upsert( + NostrEvent( + id = nostrEventId, + pubKey = proposer, + kind = 0, + tags = emptyArray(), + content = "{}", + sig = "0".repeat(128), + ) + ) + db.profileDao().upsert(Profile(publicKey = proposer, nostrEventId = nostrEventId)) + + db.chatRoomDao().upsert( + ChatRoom( + id = roomId, + userPublicKey = proposer, + subject = "#Ekklesia", + description = SharedKeyDerivation.describe("Admins of the group."), + mlsGroupState = null, + ) + ) + + if (withKey) giveTheGroupItsKey() + + db.mantraDialectDao().upsert( + MantraDialect( + id = dialectId, + publicKey = roomId, + name = "Setswana", + country = "BW", + language = "tn", + signature = "", + chatRoomId = roomId, + ) + ) + db.mantraArtifactDao().upsert( + MantraArtifact( + id = artifactId, + publicKey = roomId, + name = "To Kill a Mockingbird", + url = "https://example.test/mockingbird", + visibility = "private", + dialectId = dialectId, + license = "cc", + chatRoomId = roomId, + signature = "", + ) + ) + db.mantraArtifactVersionDao().upsert( + MantraArtifactVersion( + id = artifactVersionId, + artifactId = artifactId, + publicKey = roomId, + versionLabel = "1.0", + chatRoomId = roomId, + signature = "", + ) + ) + (0 until translations).forEach { ordinal -> + db.mantraTranslationArtifactVersionDao().upsert( + MantraTranslationArtifactVersion( + id = "translation$ordinal".padEnd(64, '0'), + publicKey = roomId, + artifactVersionId = artifactVersionId, + dialectId = dialectId, + name = "Translation $ordinal", + visibility = "private", + license = "cc", + signature = "", + chatRoomId = roomId, + ) + ) + } + + viewModel = UploadChaptersViewModel( + artifactId = artifactId, + chatRoomId = roomId, + activeUserPublicKey = proposer, + relayHint = null, + initialUIState = UploadChaptersUIState.Loading, + mantraRepository = DatabaseMantraRepository(db, scope), + chatRepository = DatabaseChatRepository(db, scope), + frostSigningRepository = DatabaseFrostSigningRepository(db, scope), + ) + viewModel.initiate() + awaitLoaded() + } + + private suspend fun giveTheGroupItsKey() { + db.dkgSessionDao().upsert( + DkgSession( + id = ceremonyId, + chatRoomId = roomId, + coordinatorPublicKey = proposer, + userPublicKey = proposer, + threshold = threshold, + participantCount = participants, + stage = DkgRitualStage.COMPLETE, + hostPublicKey = hostKeys[0], + round1Random = "1".repeat(64), + round2AuxRandom = "2".repeat(64), + thresholdPublicKey = thresholdPublicKey, + secretShare = keyMaterial.secretShares[0].value.toHex(), + publicShares = keyMaterial.publicShares.joinToString(",") { it.value.toHex() }, + ) + ) + members.forEachIndexed { index, member -> + db.dkgSessionDao().upsert( + DkgParticipantMessage( + sessionId = ceremonyId, + participantPublicKey = member, + kind = DkgRitualEvents.HOST_KEY, + payload = hostKeys[index], + ) + ) + } + } + + private suspend fun awaitLoaded(): UploadChaptersUIState.Loaded = + withTimeout(10_000) { + var state = viewModel.uiState.value + while (state !is UploadChaptersUIState.Loaded) { + kotlinx.coroutines.delay(10) + state = viewModel.uiState.value + } + state + } + + private val loaded: UploadChaptersUIState.Loaded + get() = viewModel.uiState.value as UploadChaptersUIState.Loaded + + private fun file(name: String, text: String) = + PickedFile(name = name, bytes = text.encodeToByteArray()) + + /** A chapter of [paragraphs] paragraphs, as a file would hold it. */ + private fun chapterFile(ordinal: Int, paragraphs: Int = 2) = file( + name = "chapter-$ordinal.md", + text = (1..paragraphs).joinToString("\n\n") { "Paragraph $it of chapter $ordinal." }, + ) + + /** Proposes, and waits for the pass to finish. */ + private suspend fun propose(): Pair { + val done = CompletableDeferred>() + viewModel.propose( + localChatRoom = loaded.localChatRoom, + artifactVersion = loaded.artifactVersion!!, + ) { proposed, failed -> done.complete(proposed to failed) } + return withTimeout(30_000) { done.await() } + } + + private suspend fun itemEvents(sessionId: String): List = + db.frostSigningSessionDao().getItems(sessionId).map { Event.fromJson(it.unsignedEventJson) } + + private suspend fun sessionsInRoom(): List = + db.frostSigningSessionDao().getSessionsForChatRoom(roomId).map { it.id } + + @Test + fun `five files become five sessions, one chapter each, numbered in order`() = runBlocking { + openDevice() + + viewModel.addFiles((1..5).map { chapterFile(it, paragraphs = 3) }) + assertEquals(5, loaded.chapters.size) + + val (proposed, failed) = propose() + + assertEquals(5 to 0, proposed to failed) + + val chapterSessions = loaded.chapters.map { it.sessionId!! } + assertEquals(5, chapterSessions.toSet().size) + + // Each session is one chapter leading its own chunks -- never two + // chapters sharing a batch, which is the whole shape of this feature. + val chapters = chapterSessions.map { session -> + val events = itemEvents(session) + assertEquals(ChapterEvent.KIND, events.first().kind) + assertTrue(events.drop(1).all { it.kind == ChunkEvent.KIND }) + assertEquals(4, events.size) // the chapter and its three chunks + ChapterEvent( + events.first().id, + events.first().pubKey, + events.first().createdAt, + events.first().tags, + events.first().content, + events.first().sig, + ) + } + + // Contiguous from zero, which the old `.size` numbering could not do: + // nothing is signed, so the table is empty for all five. + assertEquals(listOf(0, 1, 2, 3, 4), chapters.map { it.index() }) + assertEquals( + (1..5).map { "chapter-$it" }, + chapters.map { it.name() }, + ) + } + + @Test + fun `each chapter's chunks are the blocks of its own file`() = runBlocking { + openDevice() + + val text = "# A heading\n\nFirst paragraph.\n\n- one\n- two\n" + viewModel.addFiles(listOf(file("ch.md", text))) + propose() + + val events = itemEvents(loaded.chapters.single().sessionId!!) + val chunks = events.drop(1).map { it.content } + + assertEquals(Markdown.splitChunks(text), chunks) + // And the block markers are off, which is what Phase 8 changed. + assertEquals(listOf("A heading", "First paragraph.", "one", "two"), chunks) + } + + @Test + fun `the whole upload asks for one scaffolding session, not one per chapter`() = runBlocking { + openDevice(translations = 3) + + viewModel.addFiles((1..4).map { chapterFile(it) }) + propose() + + val chapterSessions = loaded.chapters.map { it.sessionId!! }.toSet() + val scaffolding = sessionsInRoom().filterNot { it in chapterSessions } + + assertEquals(1, scaffolding.size) + + val rows = itemEvents(scaffolding.single()) + assertEquals(12, rows.size) // four chapters x three dialects + assertTrue(rows.all { it.kind == TranslationChapterEvent.KIND }) + } + + @Test + fun `a pass that fails leaves every row listed with its reason and retryable`() = runBlocking { + // No shared key, so every proposal throws inside the view model. This is + // the reachable way to half-fail a pass: the caps are checked in the + // form, so no input gets past the review and then fails the manager. + openDevice(withKey = false) + + viewModel.addFiles((1..3).map { chapterFile(it) }) + + val (proposed, failed) = propose() + assertEquals(0 to 3, proposed to failed) + + // Nothing proposed, nothing lost, and each row says why. + assertEquals(emptyList(), sessionsInRoom()) + assertTrue(loaded.chapters.all { it.sessionId == null }) + assertTrue(loaded.chapters.all { it.problem == UploadProblem.ProposalFailed }) + + // ProposalFailed does not block, because a retry is a new proposal -- + // which is the only correct way to retry a FROST session. + assertEquals(3, loaded.proposable.size) + } + + @Test + fun `retrying after a failure proposes the rest and numbers them from zero`() = runBlocking { + openDevice(withKey = false) + + viewModel.addFiles((1..3).map { chapterFile(it) }) + propose() + + giveTheGroupItsKey() + + val (proposed, failed) = propose() + + assertEquals(3 to 0, proposed to failed) + assertTrue(loaded.chapters.all { it.problem == null }) + + val indices = loaded.chapters.map { row -> + val lead = itemEvents(row.sessionId!!).first() + ChapterEvent( + lead.id, lead.pubKey, lead.createdAt, lead.tags, lead.content, lead.sig + ).index() + } + + // The failed pass spent no indices, because it created nothing. + assertEquals(listOf(0, 1, 2), indices) + } + + @Test + fun `a row already proposed is not proposed again`() = runBlocking { + openDevice() + + viewModel.addFiles(listOf(chapterFile(1))) + propose() + val first = loaded.chapters.single().sessionId + + // Nothing left to do, so a second tap proposes nothing rather than + // asking the group to sign the same chapter twice. + val (proposed, failed) = propose() + + assertEquals(0 to 0, proposed to failed) + assertEquals(first, loaded.chapters.single().sessionId) + assertEquals(1, sessionsInRoom().size) + } + + @Test + fun `a file that is not text is never proposed and does not stop the others`() = runBlocking { + openDevice() + + viewModel.addFiles( + listOf( + chapterFile(1), + PickedFile("cover.png", byteArrayOf(0x89.toByte(), 0x50, 0x4E, 0x47, 0x00)), + chapterFile(2), + ) + ) + + val (proposed, failed) = propose() + + // Two proposed, and the third is not a failure -- it was never offered. + assertEquals(2 to 0, proposed to failed) + assertEquals(UploadProblem.NotText, loaded.chapters[1].problem) + assertNull(loaded.chapters[1].sessionId) + assertEquals(2, loaded.chapters.count { it.sessionId != null }) + } + + @Test + fun `the soft cap is on the upload, not on the artifact`() = runBlocking { + openDevice() + + viewModel.addFiles((1..UploadChaptersViewModel.MAX_FILES_PER_UPLOAD + 5).map { chapterFile(it) }) + + // The extra files are dropped at the door rather than listed and + // refused: a limit on how much of the room's attention one tap spends. + assertEquals(UploadChaptersViewModel.MAX_FILES_PER_UPLOAD, loaded.chapters.size) + } +} diff --git a/docs/chapter-uploads.md b/docs/chapter-uploads.md index ba1d81cf..8b6471f2 100644 --- a/docs/chapter-uploads.md +++ b/docs/chapter-uploads.md @@ -5,10 +5,19 @@ Picking files off a device and having a group sign each one into an artifact as [mantra.press](https://code.sigidli.com/mantra/mantra.press) does it over one HTTP POST, except that here nobody is authoring anything on their own. -**Not built.** This is a plan. The constraint that shapes every phase is stated -first, because the web app's shape — N files in, N chapters out, one -transaction, one redirect — is the one shape this app cannot have, and every -phase below is a consequence of that rather than a feature. +**Built**, all nine phases, one commit each, and the phases below are kept as +written — they are the reasoning, and the code is easier to read against the +argument it came from than against a summary of itself. Where the +implementation chose differently from the plan the section says so, and there +are four such places worth knowing about: the picker is three actuals rather +than FileKit, the byte cap is a function rather than a constant and lives on the +manager rather than the chapter, the scaffolding fix turned out to repair a +latent bug as well, and Phase 6 found all three of its worries already answered. + +The constraint that shapes every phase is stated first, because the web app's +shape — N files in, N chapters out, one transaction, one redirect — is the one +shape this app cannot have, and every phase below is a consequence of that +rather than a feature. --- @@ -961,6 +970,25 @@ file 4 leaves files 1–3 proposed and recoverable rather than rolled back. `jvmTest`, not `commonTest`, because it is Room-backed; `./gradlew :composeApp:jvmTest`, per the project's build notes. +**What the end-to-end test found**, which is the reason it was worth writing +rather than assuming the parts composed: `propose` with nothing proposable +returned without ever calling its callback. The FAB is disabled in that case so +the screen never reached it, but a callback that sometimes never fires is a trap +for the next caller. It now fires `onDone(0, 0)`. + +**And one thing it could not test, for a good reason.** The plan asked for "a +failure on file 4 leaves files 1–3 proposed and recoverable". That is +unreachable by *input*: both caps are checked in the form against the same +functions the manager enforces, and `ChapterProposalSizeTest` asserts the form's +estimate is never under the manager's measurement — so no file can pass the +review and then fail the proposal. It is reachable by *state*, which is what the +test does instead: a room with no shared key fails all three, every row says so +and stays retryable, and a second pass after the key arrives proposes all three +numbered from zero. The failed pass spent no indices, because it created +nothing. + +Total: 995 tests, and the counts per phase are in each commit. + Rollout needs no flag. The chapter and chunk kinds, their tags and their apply branches are unchanged, so an older build receives an uploaded chapter as the chapter it is. Two things do change what a peer sees, and neither needs