From 582db3814f4ec08e9331e1afacb44cd72b59d192 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sat, 15 Aug 2026 02:33:59 +0200 Subject: [PATCH 01/14] fix(support): finish interrupted report cancellation --- .../unreleased/351-direct-support-intake.md | 2 +- .../app/JvmSupportIntakeTest.kt | 63 +++++++++++++------ .../nextcloudnative/app/JvmSupportIntake.kt | 53 +++++++++++++--- 3 files changed, 90 insertions(+), 28 deletions(-) diff --git a/changes/unreleased/351-direct-support-intake.md b/changes/unreleased/351-direct-support-intake.md index 90c64debb..1c5a2d7b8 100644 --- a/changes/unreleased/351-direct-support-intake.md +++ b/changes/unreleased/351-direct-support-intake.md @@ -4,4 +4,4 @@ pull: none platforms: android, desktop user-facing: yes -Send a reviewed, privacy-filtered diagnostic report directly to Obiente Support while retaining the option to save a local copy. +Send a reviewed, privacy-filtered diagnostic report directly to Obiente Support, retain a local-copy option, and finish cancellation after the service confirms that an interrupted upload created no private report. diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt index b9fc39f99..b9d4861c4 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt @@ -663,16 +663,8 @@ class JvmSupportIntakeTest { withTimeout(5_000) { submission.join() } assertEquals("GET", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) - assertIs(fixture.intake.states().value) - assertTrue(File(fixture.temporaryRoot, "pending.json").isFile) - fixture.server.enqueue(receiptResponse(fixture.statusUrl)) - fixture.server.enqueue(MockResponse.Builder().code(200).body("{}").build()) - - fixture.intake.retry() - assertIs(fixture.intake.states().value) - assertEquals("GET", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) - assertEquals("DELETE", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) + assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) } Unit } @@ -693,7 +685,7 @@ class JvmSupportIntakeTest { fixture.server.enqueue(MockResponse.Builder().code(404).build()) withTimeout(5_000) { submission.join() } - assertIs(fixture.intake.states().value) + assertIs(fixture.intake.states().value) } Unit } @@ -972,7 +964,7 @@ class JvmSupportIntakeTest { } @Test - fun cancellingAmbiguousSubmissionRequiresDeletionReconciliation() = runBlocking { + fun cancellingAmbiguousSubmissionFinishesAfterConfirmedAbsence() = runBlocking { testFixture().use { fixture -> fixture.server.enqueue(MockResponse.Builder().onResponseStart(SocketEffect.CloseSocket()).build()) fixture.server.enqueue(MockResponse.Builder().code(503).build()) @@ -987,13 +979,6 @@ class JvmSupportIntakeTest { fixture.server.enqueue(MockResponse.Builder().code(404).build()) fixture.intake.retry() - assertIs(fixture.intake.states().value) - assertTrue(File(fixture.temporaryRoot, "pending.json").isFile) - fixture.server.enqueue(receiptResponse(fixture.statusUrl)) - fixture.server.enqueue(MockResponse.Builder().code(200).body("{}").build()) - - fixture.intake.retry() - assertIs(fixture.intake.states().value) assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) } @@ -1057,6 +1042,48 @@ class JvmSupportIntakeTest { } } + @Test + fun cancellationRetryFinishesAfterBoundedConfirmedAbsence() = runBlocking { + testFixture( + cancellationReconcileWindowMillis = 0L, + cancellationReconcilePollMillis = 1L, + ).use { fixture -> + fixture.server.enqueue( + receiptResponse(fixture.statusUrl).newBuilder().headersDelay(10, TimeUnit.SECONDS).build(), + ) + fixture.server.enqueue(MockResponse.Builder().onResponseStart(SocketEffect.CloseSocket()).build()) + + val submission = launch(Dispatchers.Default) { + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + } + requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertTrue(fixture.intake.cancel()) + submission.join() + + assertIs(fixture.intake.states().value) + val descriptor = File(fixture.temporaryRoot, "pending.json") + assertTrue(descriptor.isFile) + fixture.intake.close() + descriptor.writeText( + descriptor.readText().replace( + Regex(",\"cancellationRequestedAtEpochMillis\":\\d+"), + "", + ), + ) + val restored = fixture.newIntake() + assertIs(restored.states().value) + fixture.server.enqueue(MockResponse.Builder().code(404).build()) + + restored.retry() + + assertIs(restored.states().value) + val reconciliation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("GET", reconciliation.method) + assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) + restored.close() + } + } + @Test fun cancellationStillDeletesWhenReceiptPersistenceFails() = runBlocking { var directorySyncs = 0 diff --git a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt index 057543398..ba058337f 100644 --- a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt +++ b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt @@ -274,6 +274,10 @@ class JvmSupportIntake( } if (submission.cancellationPending) { cancellationRequested.set(true) + publishState( + SupportDiagnosticsSubmissionState.Cancelling, + submission.originAccountIdentity, + ) val receipt = submission.receipt if (receipt == null) { reconcileAfterAmbiguousResult( @@ -404,6 +408,9 @@ class JvmSupportIntake( } submission.cancellationPending = true submission.outcomeAmbiguous = true + submission.cancellationRequestedAtEpochMillis = + submission.cancellationRequestedAtEpochMillis + ?: System.currentTimeMillis().coerceAtLeast(0L) val cancellationPersisted = persistPendingSafely(submission) val call = activeCall.getAndSet(null) if (!cancellationPersisted) { @@ -773,6 +780,7 @@ class JvmSupportIntake( .get() .build() var cancellationDeadlineNanos: Long? = null + var cancellationDeadlineEpochMillis: Long? = null while (true) { val call = client.newCall(request) if (!registerActiveCall(submission, call, allowCancellationRequested = true)) { @@ -818,23 +826,37 @@ class JvmSupportIntake( } responseCode == 404 && cancellationRequested.get() -> { val nowNanos = System.nanoTime() + val nowEpochMillis = System.currentTimeMillis().coerceAtLeast(0L) + val requestedAtEpochMillis = submission.cancellationRequestedAtEpochMillis + ?: (submission.latestUploadAttemptAtEpochMillis ?: nowEpochMillis) + .coerceAtMost(nowEpochMillis) + .also { requestedAt -> + submission.cancellationRequestedAtEpochMillis = requestedAt + } + val deadlineEpochMillis = cancellationDeadlineEpochMillis + ?: requestedAtEpochMillis.saturatingAdd(cancellationReconcileWindowMillis) + .also { cancellationDeadlineEpochMillis = it } val deadlineNanos = cancellationDeadlineNanos ?: nowNanos.saturatingAdd(cancellationReconcileWindowMillis * NANOS_PER_MILLISECOND) .also { cancellationDeadlineNanos = it } - val remainingNanos = deadlineNanos - nowNanos - if (remainingNanos > 0L) { + val remainingEpochMillis = (deadlineEpochMillis - nowEpochMillis).coerceAtLeast(0L) + val remainingMonotonicMillis = ((deadlineNanos - nowNanos) / NANOS_PER_MILLISECOND) + .coerceAtLeast(0L) + val remainingMillis = minOf(remainingEpochMillis, remainingMonotonicMillis) + if (remainingMillis > 0L) { val delayMillis = minOf( cancellationReconcilePollMillis, - (remainingNanos / NANOS_PER_MILLISECOND).coerceAtLeast(1L), + remainingMillis, ) delay(delayMillis) continue } - retainForRetry( - submission, - "Support has not confirmed receipt yet. Retry again to finish deleting the private report safely.", - ambiguous = true, - ) + // The service handles report creation synchronously and exposes the committed + // receipt through the idempotency key. Once the bounded server-processing + // window has elapsed and reconciliation still returns 404, there is no remote + // capability to delete. Honor the user's cancellation and remove the retained + // local report instead of trapping it in an endless retry loop. + finishCancelled(submission) return } responseCode == 404 -> { @@ -886,6 +908,9 @@ class JvmSupportIntake( ) submission.cancellationPending = true submission.outcomeAmbiguous = true + submission.cancellationRequestedAtEpochMillis = + submission.cancellationRequestedAtEpochMillis + ?: System.currentTimeMillis().coerceAtLeast(0L) submission.receipt = receipt persistPendingSafely(submission) val capability = statusUrl.pathSegments.last() @@ -1262,6 +1287,7 @@ class JvmSupportIntake( cancellationPending = submission.cancellationPending, outcomeAmbiguous = submission.outcomeAmbiguous, latestUploadAttemptAtEpochMillis = submission.latestUploadAttemptAtEpochMillis, + cancellationRequestedAtEpochMillis = submission.cancellationRequestedAtEpochMillis, retryNotBeforeEpochMillis = submission.retryNotBeforeEpochMillis, receipt = submission.receipt, ), @@ -1371,6 +1397,10 @@ class JvmSupportIntake( require(persisted.originAccountIdentity.matches(SUPPORT_ACCOUNT_IDENTITY_PATTERN)) require(persisted.createdAtEpochMillis >= 0L) require(persisted.latestUploadAttemptAtEpochMillis == null || persisted.latestUploadAttemptAtEpochMillis >= 0L) + require( + persisted.cancellationRequestedAtEpochMillis == null || + persisted.cancellationRequestedAtEpochMillis >= 0L, + ) val nowEpochMillis = System.currentTimeMillis() val retryNotBeforeEpochMillis = persisted.retryNotBeforeEpochMillis?.takeIf { deadline -> deadline <= nowEpochMillis.saturatingAdd(MAX_SUPPORT_RETRY_AFTER_MILLIS) @@ -1421,6 +1451,7 @@ class JvmSupportIntake( cancellationPending = persisted.cancellationPending, outcomeAmbiguous = persisted.outcomeAmbiguous, latestUploadAttemptAtEpochMillis = persisted.latestUploadAttemptAtEpochMillis, + cancellationRequestedAtEpochMillis = persisted.cancellationRequestedAtEpochMillis, retryNotBeforeEpochMillis = retryNotBeforeEpochMillis, receipt = persisted.receipt, ) @@ -1608,6 +1639,7 @@ class JvmSupportIntake( var cancellationPending: Boolean = false, var outcomeAmbiguous: Boolean = false, var latestUploadAttemptAtEpochMillis: Long? = null, + var cancellationRequestedAtEpochMillis: Long? = null, var retryNotBeforeEpochMillis: Long? = null, var receipt: SupportIntakeReceipt? = null, ) { @@ -1648,6 +1680,7 @@ class JvmSupportIntake( val cancellationPending: Boolean = false, val outcomeAmbiguous: Boolean = true, val latestUploadAttemptAtEpochMillis: Long? = null, + val cancellationRequestedAtEpochMillis: Long? = null, val retryNotBeforeEpochMillis: Long? = null, val receipt: SupportIntakeReceipt? = null, ) @@ -1919,7 +1952,9 @@ private const val SUPPORT_RECOVERY_MAX_AGE_MILLIS = 30L * 24L * 60L * 60L * 1_00 private const val SUPPORT_SERVER_RETENTION_MAX_AGE_MILLIS = 30L * 24L * 60L * 60L * 1_000L private const val SUPPORT_RECEIPT_CLOCK_SKEW_MILLIS = 5L * 60L * 1_000L private const val SUPPORT_DESCRIPTOR_DELETE_RETRY_MILLIS = 60L * 1_000L -private const val SUPPORT_CANCELLATION_RECONCILE_WINDOW_MILLIS = 10L * 1_000L +// The support service accepts request bodies synchronously with a 30-second read timeout. Wait +// beyond that bound before treating repeated 404 reconciliation responses as confirmed absence. +private const val SUPPORT_CANCELLATION_RECONCILE_WINDOW_MILLIS = 35L * 1_000L private const val SUPPORT_CANCELLATION_RECONCILE_POLL_MILLIS = 500L private const val MAX_CANCELLATION_RECONCILE_WINDOW_MILLIS = 60L * 1_000L private const val NANOS_PER_MILLISECOND = 1_000_000L From 0f59e29f053d54f39a0715e6a8378db6582531dd Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sat, 15 Aug 2026 02:35:23 +0200 Subject: [PATCH 02/14] chore(changelog): link support cancellation PR --- changes/unreleased/351-direct-support-intake.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes/unreleased/351-direct-support-intake.md b/changes/unreleased/351-direct-support-intake.md index 1c5a2d7b8..0c762d721 100644 --- a/changes/unreleased/351-direct-support-intake.md +++ b/changes/unreleased/351-direct-support-intake.md @@ -1,6 +1,6 @@ category: feature issue: 351 -pull: none +pull: 386 platforms: android, desktop user-facing: yes From b7bfc7a167e4713dc26cc2c36f3598d99ee690e4 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sat, 15 Aug 2026 02:40:03 +0200 Subject: [PATCH 03/14] fix(changelog): add support cancellation fragment --- changes/unreleased/351-direct-support-intake.md | 4 ++-- changes/unreleased/386-support-cancellation-recovery.md | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 changes/unreleased/386-support-cancellation-recovery.md diff --git a/changes/unreleased/351-direct-support-intake.md b/changes/unreleased/351-direct-support-intake.md index 0c762d721..90c64debb 100644 --- a/changes/unreleased/351-direct-support-intake.md +++ b/changes/unreleased/351-direct-support-intake.md @@ -1,7 +1,7 @@ category: feature issue: 351 -pull: 386 +pull: none platforms: android, desktop user-facing: yes -Send a reviewed, privacy-filtered diagnostic report directly to Obiente Support, retain a local-copy option, and finish cancellation after the service confirms that an interrupted upload created no private report. +Send a reviewed, privacy-filtered diagnostic report directly to Obiente Support while retaining the option to save a local copy. diff --git a/changes/unreleased/386-support-cancellation-recovery.md b/changes/unreleased/386-support-cancellation-recovery.md new file mode 100644 index 000000000..e59f6ebaf --- /dev/null +++ b/changes/unreleased/386-support-cancellation-recovery.md @@ -0,0 +1,7 @@ +category: fix +issue: 351 +pull: 386 +platforms: android, desktop +user-facing: yes + +Finish cancelling a private support report after Obiente Support confirms that an interrupted upload created no report to delete. From bc016fc0474bebf9d68653fe729b240820eb7524 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sat, 15 Aug 2026 02:55:41 +0200 Subject: [PATCH 04/14] fix(support): require terminal cancellation --- .../386-support-cancellation-recovery.md | 2 +- .../app/JvmSupportIntakeTest.kt | 248 +++++++--------- .../nextcloudnative/app/JvmSupportIntake.kt | 278 +++++------------- 3 files changed, 178 insertions(+), 350 deletions(-) diff --git a/changes/unreleased/386-support-cancellation-recovery.md b/changes/unreleased/386-support-cancellation-recovery.md index e59f6ebaf..c31977ed0 100644 --- a/changes/unreleased/386-support-cancellation-recovery.md +++ b/changes/unreleased/386-support-cancellation-recovery.md @@ -4,4 +4,4 @@ pull: 386 platforms: android, desktop user-facing: yes -Finish cancelling a private support report after Obiente Support confirms that an interrupted upload created no report to delete. +Finish cancelling a private support report only after Obiente Support confirms that its upload key is terminally cancelled. diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt index b9d4861c4..6af9af3e8 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt @@ -653,16 +653,20 @@ class JvmSupportIntakeTest { fixture.server.enqueue( receiptResponse(fixture.statusUrl).newBuilder().headersDelay(10, TimeUnit.SECONDS).build(), ) - fixture.server.enqueue(MockResponse.Builder().code(404).build()) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) val submission = launch(Dispatchers.Default) { fixture.intake.submit("A refresh failed.", "nightly", emptyList()) } - assertEquals("POST", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("POST", upload.method) assertFalse(fixture.intake.cancel()) withTimeout(5_000) { submission.join() } - assertEquals("GET", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) + val cancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", cancellation.method) + assertEquals("/api/v1/receipts", cancellation.url.encodedPath) + assertEquals(upload.headers["Idempotency-Key"], cancellation.headers["Idempotency-Key"]) assertIs(fixture.intake.states().value) assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) } @@ -683,9 +687,12 @@ class JvmSupportIntakeTest { assertTrue(fixture.intake.cancel()) assertIs(fixture.intake.states().value) - fixture.server.enqueue(MockResponse.Builder().code(404).build()) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) withTimeout(5_000) { submission.join() } assertIs(fixture.intake.states().value) + val cancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", cancellation.method) + assertEquals("/api/v1/receipts", cancellation.url.encodedPath) } Unit } @@ -964,7 +971,7 @@ class JvmSupportIntakeTest { } @Test - fun cancellingAmbiguousSubmissionFinishesAfterConfirmedAbsence() = runBlocking { + fun cancellingAmbiguousSubmissionUsesAuthoritativeServerTombstone() = runBlocking { testFixture().use { fixture -> fixture.server.enqueue(MockResponse.Builder().onResponseStart(SocketEffect.CloseSocket()).build()) fixture.server.enqueue(MockResponse.Builder().code(503).build()) @@ -972,20 +979,22 @@ class JvmSupportIntakeTest { fixture.intake.submit("A refresh failed.", "nightly", emptyList()) assertIs(fixture.intake.states().value) + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) assertTrue(fixture.intake.cancel()) - assertIs(fixture.intake.states().value) - assertTrue(File(fixture.temporaryRoot, "pending.json").isFile) - - fixture.server.enqueue(MockResponse.Builder().code(404).build()) - fixture.intake.retry() assertIs(fixture.intake.states().value) + val cancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", cancellation.method) + assertEquals("/api/v1/receipts", cancellation.url.encodedPath) + assertEquals(upload.headers["Idempotency-Key"], cancellation.headers["Idempotency-Key"]) assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) } } @Test - fun serverFailureRemainsAmbiguousUntilDiscardReconcilesAndDeletes() = runBlocking { + fun serverFailureRemainsAmbiguousUntilCancellationIsConfirmed() = runBlocking { testFixture().use { fixture -> fixture.server.enqueue(MockResponse.Builder().code(503).build()) @@ -994,32 +1003,23 @@ class JvmSupportIntakeTest { val retryable = assertIs(fixture.intake.states().value) assertTrue(retryable.outcomeAmbiguous) val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) assertTrue(fixture.intake.cancel()) - fixture.server.enqueue(receiptResponse(fixture.statusUrl)) - fixture.server.enqueue(MockResponse.Builder().code(200).body("{}").build()) - - fixture.intake.retry() assertIs(fixture.intake.states().value) - val reconciliation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - val deletion = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - assertEquals(upload.headers["Idempotency-Key"], reconciliation.headers["Idempotency-Key"]) - assertEquals("GET", reconciliation.method) - assertEquals("DELETE", deletion.method) + val cancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals(upload.headers["Idempotency-Key"], cancellation.headers["Idempotency-Key"]) + assertEquals("DELETE", cancellation.method) + assertEquals("/api/v1/receipts", cancellation.url.encodedPath) assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) } } @Test - fun cancellationRechecksInitialAbsenceAndDeletesLateReceipt() = runBlocking { - testFixture( - cancellationReconcileWindowMillis = 1_000L, - cancellationReconcilePollMillis = 1L, - ).use { fixture -> + fun cancellationDoesNotPollReceiptAbsenceBeforeDiscardingRecovery() = runBlocking { + testFixture().use { fixture -> fixture.server.enqueue(receiptResponse(fixture.statusUrl).newBuilder().headersDelay(10, TimeUnit.SECONDS).build()) - fixture.server.enqueue(MockResponse.Builder().code(404).build()) - fixture.server.enqueue(receiptResponse(fixture.statusUrl)) - fixture.server.enqueue(MockResponse.Builder().code(200).body("{}").build()) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) val submission = launch(Dispatchers.Default) { fixture.intake.submit("A refresh failed.", "nightly", emptyList()) @@ -1029,25 +1029,18 @@ class JvmSupportIntakeTest { submission.join() assertIs(fixture.intake.states().value) - val firstReconcile = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - val secondReconcile = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - val deletion = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - assertEquals(upload.headers["Idempotency-Key"], firstReconcile.headers["Idempotency-Key"]) - assertEquals(upload.headers["Idempotency-Key"], secondReconcile.headers["Idempotency-Key"]) - assertEquals("GET", firstReconcile.method) - assertEquals("GET", secondReconcile.method) - assertEquals("DELETE", deletion.method) - assertTrue(deletion.url.encodedPath.startsWith("/api/v1/reports/")) + val cancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals(upload.headers["Idempotency-Key"], cancellation.headers["Idempotency-Key"]) + assertEquals("DELETE", cancellation.method) + assertEquals("/api/v1/receipts", cancellation.url.encodedPath) + assertEquals(2, fixture.server.requestCount) assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) } } @Test - fun cancellationRetryFinishesAfterBoundedConfirmedAbsence() = runBlocking { - testFixture( - cancellationReconcileWindowMillis = 0L, - cancellationReconcilePollMillis = 1L, - ).use { fixture -> + fun cancellationRetryWaitsForAuthoritativeTerminalResult() = runBlocking { + testFixture().use { fixture -> fixture.server.enqueue( receiptResponse(fixture.statusUrl).newBuilder().headersDelay(10, TimeUnit.SECONDS).build(), ) @@ -1063,49 +1056,46 @@ class JvmSupportIntakeTest { assertIs(fixture.intake.states().value) val descriptor = File(fixture.temporaryRoot, "pending.json") assertTrue(descriptor.isFile) + val firstCancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", firstCancellation.method) fixture.intake.close() - descriptor.writeText( - descriptor.readText().replace( - Regex(",\"cancellationRequestedAtEpochMillis\":\\d+"), - "", - ), - ) val restored = fixture.newIntake() assertIs(restored.states().value) - fixture.server.enqueue(MockResponse.Builder().code(404).build()) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) restored.retry() assertIs(restored.states().value) - val reconciliation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - assertEquals("GET", reconciliation.method) + val retryCancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", retryCancellation.method) + assertEquals("/api/v1/receipts", retryCancellation.url.encodedPath) + assertEquals( + firstCancellation.headers["Idempotency-Key"], + retryCancellation.headers["Idempotency-Key"], + ) assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) restored.close() } } @Test - fun cancellationStillDeletesWhenReceiptPersistenceFails() = runBlocking { - var directorySyncs = 0 - testFixture( - directorySync = { - directorySyncs += 1 - if (directorySyncs == 5) throw IOException("Synthetic receipt persistence failure.") - }, - ).use { fixture -> - fixture.server.enqueue(receiptResponse(fixture.statusUrl).newBuilder().headersDelay(10, TimeUnit.SECONDS).build()) - fixture.server.enqueue(receiptResponse(fixture.statusUrl)) - fixture.server.enqueue(MockResponse.Builder().code(200).body("{}").build()) - val submission = launch(Dispatchers.Default) { - fixture.intake.submit("A refresh failed.", "nightly", emptyList()) - } + fun cancellationRetainsRecoveryUntilTerminalNoContent() = runBlocking { + testFixture().use { fixture -> + fixture.server.enqueue(MockResponse.Builder().code(503).build()) + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - + fixture.server.enqueue(MockResponse.Builder().code(200).body("{}").build()) assertTrue(fixture.intake.cancel()) - submission.join() + + assertIs(fixture.intake.states().value) + assertTrue(File(fixture.temporaryRoot, "pending.json").isFile) + val nonTerminal = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", nonTerminal.method) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) + + fixture.intake.retry() assertIs(fixture.intake.states().value) - assertEquals("GET", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) assertEquals("DELETE", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) } @@ -1249,23 +1239,21 @@ class JvmSupportIntakeTest { submission.join() assertIs(fixture.intake.states().value) - val firstReconciliation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - assertEquals("GET", firstReconciliation.method) + val firstCancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", firstCancellation.method) fixture.intake.close() val restored = fixture.newIntake() assertIs(restored.states().value) - fixture.server.enqueue(receiptResponse(fixture.statusUrl)) - fixture.server.enqueue(MockResponse.Builder().code(200).body("{}").build()) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) restored.retry() assertIs(restored.states().value) - val retryReconciliation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - val deletion = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - assertEquals(upload.headers["Idempotency-Key"], retryReconciliation.headers["Idempotency-Key"]) - assertEquals("GET", retryReconciliation.method) - assertEquals("DELETE", deletion.method) + val retryCancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals(upload.headers["Idempotency-Key"], retryCancellation.headers["Idempotency-Key"]) + assertEquals("DELETE", retryCancellation.method) + assertEquals("/api/v1/receipts", retryCancellation.url.encodedPath) assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) } } @@ -1367,85 +1355,52 @@ class JvmSupportIntakeTest { } @Test - fun retriesPersistedDeletionCapabilityWithoutResubmitting() = runBlocking { + fun retriesPersistedCancellationWithoutResubmitting() = runBlocking { testFixture().use { fixture -> fixture.server.enqueue(receiptResponse(fixture.statusUrl).newBuilder().headersDelay(10, TimeUnit.SECONDS).build()) - fixture.server.enqueue(receiptResponse(fixture.statusUrl)) fixture.server.enqueue(MockResponse.Builder().code(503).build()) val submission = launch(Dispatchers.Default) { fixture.intake.submit("A refresh failed.", "nightly", emptyList()) } - requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) assertTrue(fixture.intake.cancel()) submission.join() assertIs(fixture.intake.states().value) - val reconciliation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - val failedDeletion = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - assertEquals("GET", reconciliation.method) - assertEquals("DELETE", failedDeletion.method) + val failedCancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", failedCancellation.method) + assertEquals("/api/v1/receipts", failedCancellation.url.encodedPath) + assertEquals(upload.headers["Idempotency-Key"], failedCancellation.headers["Idempotency-Key"]) fixture.intake.close() val restored = fixture.newIntake() - fixture.server.enqueue(MockResponse.Builder().code(200).body("{}").build()) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) restored.retry() assertIs(restored.states().value) - val retriedDeletion = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - assertEquals("DELETE", retriedDeletion.method) - assertEquals(4, fixture.server.requestCount) + val retriedCancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", retriedCancellation.method) + assertEquals("/api/v1/receipts", retriedCancellation.url.encodedPath) + assertEquals(upload.headers["Idempotency-Key"], retriedCancellation.headers["Idempotency-Key"]) + assertEquals(3, fixture.server.requestCount) assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) } } @Test - fun acceptedDeletionKeepsReceiptUntilStatusConfirmsRemoval() = runBlocking { + fun keepsCancellationKeyAfterTheLocalArchiveRetentionWindow() = runBlocking { testFixture().use { fixture -> fixture.server.enqueue(receiptResponse(fixture.statusUrl).newBuilder().headersDelay(10, TimeUnit.SECONDS).build()) - fixture.server.enqueue(receiptResponse(fixture.statusUrl)) - fixture.server.enqueue(MockResponse.Builder().code(202).body("{}").build()) - fixture.server.enqueue(MockResponse.Builder().code(200).body("{}").build()) - - val submission = launch(Dispatchers.Default) { - fixture.intake.submit("A refresh failed.", "nightly", emptyList()) - } - requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - assertTrue(fixture.intake.cancel()) - submission.join() - - assertIs(fixture.intake.states().value) - assertTrue(File(fixture.temporaryRoot, "pending.json").isFile) - val reconciliation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - val acceptedDeletion = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - val statusCheck = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - assertEquals("GET", reconciliation.method) - assertEquals("DELETE", acceptedDeletion.method) - assertEquals("GET", statusCheck.method) - - fixture.server.enqueue(MockResponse.Builder().code(200).body("{}").build()) - fixture.intake.retry() - - assertIs(fixture.intake.states().value) - assertEquals("DELETE", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) - assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) - } - } - - @Test - fun keepsDeletionCapabilityAfterTheLocalArchiveRetentionWindow() = runBlocking { - testFixture().use { fixture -> - fixture.server.enqueue(receiptResponse(fixture.statusUrl).newBuilder().headersDelay(10, TimeUnit.SECONDS).build()) - fixture.server.enqueue(receiptResponse(fixture.statusUrl)) fixture.server.enqueue(MockResponse.Builder().code(503).build()) val submission = launch(Dispatchers.Default) { fixture.intake.submit("A refresh failed.", "nightly", emptyList()) } - requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) assertTrue(fixture.intake.cancel()) submission.join() - requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertIs(fixture.intake.states().value) requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) fixture.intake.close() @@ -1462,30 +1417,30 @@ class JvmSupportIntakeTest { assertIs(restored.states().value) assertTrue(descriptor.isFile) assertFalse(fixture.temporaryRoot.listFiles().orEmpty().any { it.extension == "zip" }) - fixture.server.enqueue(MockResponse.Builder().code(200).body("{}").build()) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) restored.retry() assertIs(restored.states().value) - assertEquals("DELETE", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) + val cancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", cancellation.method) + assertEquals(upload.headers["Idempotency-Key"], cancellation.headers["Idempotency-Key"]) assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) } } @Test - fun restoresDeletionCapabilityWhenTheWallClockMovesBackward() = runBlocking { + fun restoresCancellationKeyWhenTheWallClockMovesBackward() = runBlocking { testFixture().use { fixture -> fixture.server.enqueue(receiptResponse(fixture.statusUrl).newBuilder().headersDelay(10, TimeUnit.SECONDS).build()) - fixture.server.enqueue(receiptResponse(fixture.statusUrl)) fixture.server.enqueue(MockResponse.Builder().code(503).build()) val submission = launch(Dispatchers.Default) { fixture.intake.submit("A refresh failed.", "nightly", emptyList()) } - requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) assertTrue(fixture.intake.cancel()) submission.join() requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) fixture.intake.close() val descriptor = File(fixture.temporaryRoot, "pending.json") @@ -1501,20 +1456,21 @@ class JvmSupportIntakeTest { assertIs(restored.states().value) assertTrue(descriptor.isFile) - fixture.server.enqueue(MockResponse.Builder().code(200).body("{}").build()) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) restored.retry() assertIs(restored.states().value) - assertEquals("DELETE", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) + val cancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", cancellation.method) + assertEquals(upload.headers["Idempotency-Key"], cancellation.headers["Idempotency-Key"]) assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) } } @Test - fun preservesLastPersistedReceiptWhenRetryStateCannotBeRewritten() = runBlocking { + fun preservesLastCancellationRecordWhenRetryStateCannotBeRewritten() = runBlocking { testFixture().use { fixture -> fixture.server.enqueue(receiptResponse(fixture.statusUrl).newBuilder().headersDelay(10, TimeUnit.SECONDS).build()) - fixture.server.enqueue(receiptResponse(fixture.statusUrl)) fixture.server.enqueue( MockResponse.Builder().code(503).headersDelay(1, TimeUnit.SECONDS).build(), ) @@ -1522,10 +1478,10 @@ class JvmSupportIntakeTest { val submission = launch(Dispatchers.Default) { fixture.intake.submit("A refresh failed.", "nightly", emptyList()) } - requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) assertTrue(fixture.intake.cancel()) - requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + val cancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", cancellation.method) val retainedRoot = File(fixture.root, "submissions-retained") Files.move(fixture.temporaryRoot.toPath(), retainedRoot.toPath()) fixture.temporaryRoot.writeText("temporarily unavailable") @@ -1535,19 +1491,21 @@ class JvmSupportIntakeTest { assertTrue(state.message.contains("could not be stored")) val descriptor = File(retainedRoot, "pending.json") assertTrue(descriptor.isFile) - assertTrue(descriptor.readText().contains("OBI-ABCDE-23456")) + assertTrue(descriptor.readText().contains(upload.headers["Idempotency-Key"].orEmpty())) assertTrue(fixture.temporaryRoot.delete()) Files.move(retainedRoot.toPath(), fixture.temporaryRoot.toPath()) fixture.intake.close() val restored = fixture.newIntake() assertIs(restored.states().value) - fixture.server.enqueue(MockResponse.Builder().code(200).body("{}").build()) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) restored.retry() assertIs(restored.states().value) - assertEquals("DELETE", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) + val retryCancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", retryCancellation.method) + assertEquals(upload.headers["Idempotency-Key"], retryCancellation.headers["Idempotency-Key"]) assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) } } @@ -1709,8 +1667,6 @@ class JvmSupportIntakeTest { privateFileDelete: (File) -> Boolean = File::delete, pendingDescriptorRead: (File) -> String = { descriptor -> descriptor.readText() }, completedDescriptorRead: (File) -> String = { descriptor -> descriptor.readText() }, - cancellationReconcileWindowMillis: Long = 0L, - cancellationReconcilePollMillis: Long = 1L, submissionStorageBlocked: Boolean = false, pendingTemporaryBeforeInitialization: Boolean = false, archiveTemporaryBeforeInitialization: Boolean = false, @@ -1775,8 +1731,6 @@ class JvmSupportIntakeTest { privateFileDelete = privateFileDelete, pendingDescriptorRead = pendingDescriptorRead, completedDescriptorRead = completedDescriptorRead, - cancellationReconcileWindowMillis = cancellationReconcileWindowMillis, - cancellationReconcilePollMillis = cancellationReconcilePollMillis, ) } @@ -1821,8 +1775,6 @@ class JvmSupportIntakeTest { val privateFileDelete: (File) -> Boolean, val pendingDescriptorRead: (File) -> String, val completedDescriptorRead: (File) -> String, - val cancellationReconcileWindowMillis: Long, - val cancellationReconcilePollMillis: Long, ) : AutoCloseable { val intake = newIntake() val statusUrl: String get() = server.url("/r/abcdefghijklmnopqrstuvwxyzABCDEFGH_12345678").toString() @@ -1846,8 +1798,6 @@ class JvmSupportIntakeTest { privateFileDelete = privateFileDelete, pendingDescriptorRead = pendingDescriptorRead, completedDescriptorRead = completedDescriptorRead, - cancellationReconcileWindowMillis = cancellationReconcileWindowMillis, - cancellationReconcilePollMillis = cancellationReconcilePollMillis, ).also { intake -> intake.setActiveAccountIdentity(TEST_ACCOUNT_IDENTITY) runBlocking { intake.awaitInitialization() } diff --git a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt index ba058337f..973165fa6 100644 --- a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt +++ b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt @@ -73,8 +73,6 @@ class JvmSupportIntake( private val completedDescriptorRead: (File) -> String = { descriptor -> descriptor.readText(Charsets.UTF_8) }, - private val cancellationReconcileWindowMillis: Long = SUPPORT_CANCELLATION_RECONCILE_WINDOW_MILLIS, - private val cancellationReconcilePollMillis: Long = SUPPORT_CANCELLATION_RECONCILE_POLL_MILLIS, ) : AutoCloseable { private val baseUrl = supportBaseUrl.toHttpUrl() private val client = client.newBuilder() @@ -109,8 +107,6 @@ class JvmSupportIntake( init { require(descriptorCleanupRetryMillis > 0L) - require(cancellationReconcileWindowMillis in 0L..MAX_CANCELLATION_RECONCILE_WINDOW_MILLIS) - require(cancellationReconcilePollMillis > 0L) scope.launch { try { val storageFailure = runCatching { preparePrivateStorage() }.exceptionOrNull() @@ -278,15 +274,7 @@ class JvmSupportIntake( SupportDiagnosticsSubmissionState.Cancelling, submission.originAccountIdentity, ) - val receipt = submission.receipt - if (receipt == null) { - reconcileAfterAmbiguousResult( - submission, - IOException("Cancellation still needs to be reconciled."), - ) - } else { - deleteCancelledReceipt(submission, receipt) - } + cancelPendingSubmission(submission) return@withContext } val waitMillis = submission.retryNotBeforeEpochMillis?.minus(System.currentTimeMillis()) ?: 0L @@ -408,9 +396,6 @@ class JvmSupportIntake( } submission.cancellationPending = true submission.outcomeAmbiguous = true - submission.cancellationRequestedAtEpochMillis = - submission.cancellationRequestedAtEpochMillis - ?: System.currentTimeMillis().coerceAtLeast(0L) val cancellationPersisted = persistPendingSafely(submission) val call = activeCall.getAndSet(null) if (!cancellationPersisted) { @@ -431,10 +416,22 @@ class JvmSupportIntake( } } if (submission != null) { - publishState(SupportDiagnosticsSubmissionState.RetryableFailure( - "Cancellation could not be confirmed. Retry safely to reconcile and delete the private report.", - outcomeAmbiguous = true, - )) + if (beginOperation()) { + try { + publishState( + SupportDiagnosticsSubmissionState.Cancelling, + submission.originAccountIdentity, + ) + cancelPendingSubmission(submission) + } finally { + endOperation() + } + } else { + publishState( + SupportDiagnosticsSubmissionState.Cancelling, + submission.originAccountIdentity, + ) + } return true } cancellationRequested.compareAndSet(true, false) @@ -739,6 +736,7 @@ class JvmSupportIntake( ambiguous = false, retryNotBeforeEpochMillis = response.retryNotBeforeEpochMillis(), ) + response.code == 410 && cancellationRequested.get() -> finishCancelled(submission) response.code in 400..499 -> finishRejected(submission, decodeProblem(responseText)) else -> retainForRetry( submission, @@ -773,105 +771,62 @@ class JvmSupportIntake( submission: PendingSubmission, uploadFailure: IOException, ) { + if (cancellationRequested.get() || submission.cancellationPending) { + cancelPendingSubmission(submission) + return + } val request = Request.Builder() .url(baseUrl.newBuilder().addPathSegments("api/v1/receipts").build()) .header("Accept", "application/json") .header("Idempotency-Key", submission.idempotencyKey) .get() .build() - var cancellationDeadlineNanos: Long? = null - var cancellationDeadlineEpochMillis: Long? = null - while (true) { - val call = client.newCall(request) - if (!registerActiveCall(submission, call, allowCancellationRequested = true)) { - call.cancel() - if (synchronized(lock) { pending === submission }) { - retainForRetry( - submission, - "The upload result still needs to be reconciled. You can retry it safely.", - ambiguous = true, - ) - } - return - } - val responseResult = try { - call.execute().use { response -> - response.code to response.readBoundedText() - } - } catch (_: IOException) { + val call = client.newCall(request) + if (!registerActiveCall(submission, call, allowCancellationRequested = true)) { + call.cancel() + if (synchronized(lock) { pending === submission }) { retainForRetry( submission, - if (cancellationRequested.get()) { - "Cancellation could not be confirmed. Reconcile the private submission before retrying." - } else uploadFailure.message?.filterSupportMetadata(MAX_SUPPORT_INTAKE_MESSAGE_LENGTH) - ?.takeIf(String::isNotBlank) - ?: "The upload result is uncertain. Check your connection before retrying.", - true, + "The upload result still needs to be reconciled. You can retry it safely.", + ambiguous = true, ) - return - } finally { - activeCall.compareAndSet(call, null) } - val (responseCode, responseText) = responseResult - when { - responseCode in 200..299 -> { - try { - finishReceived(submission, decodeReceipt(responseText)) - } catch (_: IOException) { - retainForRetry(submission, "Obiente Support returned an invalid receipt.", true) - } catch (_: IllegalArgumentException) { - retainForRetry(submission, "Obiente Support returned an invalid receipt.", true) - } - return - } - responseCode == 404 && cancellationRequested.get() -> { - val nowNanos = System.nanoTime() - val nowEpochMillis = System.currentTimeMillis().coerceAtLeast(0L) - val requestedAtEpochMillis = submission.cancellationRequestedAtEpochMillis - ?: (submission.latestUploadAttemptAtEpochMillis ?: nowEpochMillis) - .coerceAtMost(nowEpochMillis) - .also { requestedAt -> - submission.cancellationRequestedAtEpochMillis = requestedAt - } - val deadlineEpochMillis = cancellationDeadlineEpochMillis - ?: requestedAtEpochMillis.saturatingAdd(cancellationReconcileWindowMillis) - .also { cancellationDeadlineEpochMillis = it } - val deadlineNanos = cancellationDeadlineNanos - ?: nowNanos.saturatingAdd(cancellationReconcileWindowMillis * NANOS_PER_MILLISECOND) - .also { cancellationDeadlineNanos = it } - val remainingEpochMillis = (deadlineEpochMillis - nowEpochMillis).coerceAtLeast(0L) - val remainingMonotonicMillis = ((deadlineNanos - nowNanos) / NANOS_PER_MILLISECOND) - .coerceAtLeast(0L) - val remainingMillis = minOf(remainingEpochMillis, remainingMonotonicMillis) - if (remainingMillis > 0L) { - val delayMillis = minOf( - cancellationReconcilePollMillis, - remainingMillis, - ) - delay(delayMillis) - continue - } - // The service handles report creation synchronously and exposes the committed - // receipt through the idempotency key. Once the bounded server-processing - // window has elapsed and reconciliation still returns 404, there is no remote - // capability to delete. Honor the user's cancellation and remove the retained - // local report instead of trapping it in an endless retry loop. - finishCancelled(submission) - return - } - responseCode == 404 -> { - retainForRetry(submission, "The upload did not complete. You can retry it safely.", false) - return - } - else -> { - retainForRetry( - submission, - "The upload result is uncertain. Check your connection before retrying.", - true, - ) - return + return + } + val responseResult = try { + call.execute().use { response -> + response.code to response.readBoundedText() + } + } catch (_: IOException) { + retainForRetry( + submission, + uploadFailure.message?.filterSupportMetadata(MAX_SUPPORT_INTAKE_MESSAGE_LENGTH) + ?.takeIf(String::isNotBlank) + ?: "The upload result is uncertain. Check your connection before retrying.", + true, + ) + return + } finally { + activeCall.compareAndSet(call, null) + } + val (responseCode, responseText) = responseResult + when { + responseCode in 200..299 -> { + try { + finishReceived(submission, decodeReceipt(responseText)) + } catch (_: IOException) { + retainForRetry(submission, "Obiente Support returned an invalid receipt.", true) + } catch (_: IllegalArgumentException) { + retainForRetry(submission, "Obiente Support returned an invalid receipt.", true) } } + responseCode == 404 -> + retainForRetry(submission, "The upload did not complete. You can retry it safely.", false) + else -> retainForRetry( + submission, + "The upload result is uncertain. Check your connection before retrying.", + true, + ) } } @@ -889,45 +844,35 @@ class JvmSupportIntake( } } if (!submitReceivedReport) { - deleteCancelledReceipt(submission, receipt) + cancelPendingSubmission(submission, receipt) } else { finishSubmitted(submission, receipt) } } - private fun deleteCancelledReceipt(submission: PendingSubmission, receipt: SupportIntakeReceipt) { - val statusUrl = validateReceipt(receipt) - val deletionUrl = receipt.deletionUrl.toHttpUrl() - require( - deletionUrl.scheme == statusUrl.scheme && - deletionUrl.host == statusUrl.host && - deletionUrl.port == statusUrl.port && - deletionUrl.encodedPath == statusUrl.encodedPath && - deletionUrl.encodedQuery == null && - deletionUrl.fragment == null, - ) + private fun cancelPendingSubmission( + submission: PendingSubmission, + receipt: SupportIntakeReceipt? = submission.receipt, + ) { submission.cancellationPending = true submission.outcomeAmbiguous = true - submission.cancellationRequestedAtEpochMillis = - submission.cancellationRequestedAtEpochMillis - ?: System.currentTimeMillis().coerceAtLeast(0L) submission.receipt = receipt persistPendingSafely(submission) - val capability = statusUrl.pathSegments.last() val request = Request.Builder() - .url(baseUrl.newBuilder().addPathSegments("api/v1/reports").addPathSegment(capability).build()) + .url(baseUrl.newBuilder().addPathSegments("api/v1/receipts").build()) .header("Accept", "application/json") + .header("Idempotency-Key", submission.idempotencyKey) .delete() .build() if (!supportMutationsAreAllowed()) { - retainCancellationForRetry(submission, receipt, READ_ONLY_SUPPORT_MESSAGE) + retainCancellationForRetry(submission, READ_ONLY_SUPPORT_MESSAGE) return } val call = client.newCall(request) if (!registerActiveCall(submission, call, allowCancellationRequested = true)) { call.cancel() if (synchronized(lock) { pending === submission }) { - retainCancellationForRetry(submission, receipt, "Deletion still needs to be confirmed. Retry safely.") + retainCancellationForRetry(submission, "Cancellation still needs to be confirmed. Retry safely.") } return } @@ -936,71 +881,17 @@ class JvmSupportIntake( response.readBoundedText() activeCall.compareAndSet(call, null) when { - response.code in TERMINAL_DELETION_STATUS_CODES || response.code == 404 -> - finishCancelled(submission) - response.isSuccessful -> verifyDeletionAfterAccepted( - submission, - receipt, - capability, - ) + response.code == 204 -> finishCancelled(submission) else -> retainCancellationForRetry( submission, - receipt, - "Deletion could not be confirmed. Retry safely to delete the private report.", + "Obiente Support did not confirm cancellation. Retry safely; the private report remains recoverable.", ) } } } catch (_: IOException) { retainCancellationForRetry( submission, - receipt, - "Deletion could not be confirmed. Check your connection, then retry safely.", - ) - } finally { - activeCall.compareAndSet(call, null) - } - } - - private fun verifyDeletionAfterAccepted( - submission: PendingSubmission, - receipt: SupportIntakeReceipt, - capability: String, - ) { - val request = Request.Builder() - .url(baseUrl.newBuilder().addPathSegments("api/v1/reports").addPathSegment(capability).build()) - .header("Accept", "application/json") - .get() - .build() - val call = client.newCall(request) - if (!registerActiveCall(submission, call, allowCancellationRequested = true)) { - call.cancel() - if (synchronized(lock) { pending === submission }) { - retainCancellationForRetry( - submission, - receipt, - "Deletion verification was interrupted. Retry safely.", - ) - } - return - } - try { - call.execute().use { response -> - response.readBoundedText() - if (response.code == 404) { - finishCancelled(submission) - } else { - retainCancellationForRetry( - submission, - receipt, - "Deletion is still being processed. Retry safely to verify the private report was removed.", - ) - } - } - } catch (_: IOException) { - retainCancellationForRetry( - submission, - receipt, - "Deletion was accepted but could not be verified. Check your connection, then retry safely.", + "Cancellation could not be confirmed. Check your connection, then retry safely.", ) } finally { activeCall.compareAndSet(call, null) @@ -1184,12 +1075,10 @@ class JvmSupportIntake( private fun retainCancellationForRetry( submission: PendingSubmission, - receipt: SupportIntakeReceipt, message: String, ) { submission.cancellationPending = true submission.outcomeAmbiguous = true - submission.receipt = receipt synchronized(lock) { pending = submission } if (persistPendingSafely(submission)) { publishState(SupportDiagnosticsSubmissionState.RetryableFailure(message, outcomeAmbiguous = true)) @@ -1197,7 +1086,7 @@ class JvmSupportIntake( // The receipt was persisted before deletion began. Atomic replacement leaves that last // valid recovery record in place when this newer retry-state write fails. publishState(SupportDiagnosticsSubmissionState.RetryableFailure( - "Deletion was not confirmed and its updated retry state could not be stored. Keep the app open and retry.", + "Cancellation was not confirmed and its updated retry state could not be stored. Keep the app open and retry.", outcomeAmbiguous = true, )) } @@ -1287,7 +1176,6 @@ class JvmSupportIntake( cancellationPending = submission.cancellationPending, outcomeAmbiguous = submission.outcomeAmbiguous, latestUploadAttemptAtEpochMillis = submission.latestUploadAttemptAtEpochMillis, - cancellationRequestedAtEpochMillis = submission.cancellationRequestedAtEpochMillis, retryNotBeforeEpochMillis = submission.retryNotBeforeEpochMillis, receipt = submission.receipt, ), @@ -1397,10 +1285,6 @@ class JvmSupportIntake( require(persisted.originAccountIdentity.matches(SUPPORT_ACCOUNT_IDENTITY_PATTERN)) require(persisted.createdAtEpochMillis >= 0L) require(persisted.latestUploadAttemptAtEpochMillis == null || persisted.latestUploadAttemptAtEpochMillis >= 0L) - require( - persisted.cancellationRequestedAtEpochMillis == null || - persisted.cancellationRequestedAtEpochMillis >= 0L, - ) val nowEpochMillis = System.currentTimeMillis() val retryNotBeforeEpochMillis = persisted.retryNotBeforeEpochMillis?.takeIf { deadline -> deadline <= nowEpochMillis.saturatingAdd(MAX_SUPPORT_RETRY_AFTER_MILLIS) @@ -1451,7 +1335,6 @@ class JvmSupportIntake( cancellationPending = persisted.cancellationPending, outcomeAmbiguous = persisted.outcomeAmbiguous, latestUploadAttemptAtEpochMillis = persisted.latestUploadAttemptAtEpochMillis, - cancellationRequestedAtEpochMillis = persisted.cancellationRequestedAtEpochMillis, retryNotBeforeEpochMillis = retryNotBeforeEpochMillis, receipt = persisted.receipt, ) @@ -1557,7 +1440,7 @@ class JvmSupportIntake( } ?: pendingSubmission?.let { submission -> SupportDiagnosticsSubmissionState.RetryableFailure( if (submission.cancellationPending) { - "Cancellation was interrupted. Retry safely to reconcile and delete the private report." + "Cancellation was interrupted. Retry safely to obtain terminal confirmation from Obiente Support." } else if (submission.archive == null) { "Private report preparation was interrupted. You can retry it safely." } else { @@ -1639,7 +1522,6 @@ class JvmSupportIntake( var cancellationPending: Boolean = false, var outcomeAmbiguous: Boolean = false, var latestUploadAttemptAtEpochMillis: Long? = null, - var cancellationRequestedAtEpochMillis: Long? = null, var retryNotBeforeEpochMillis: Long? = null, var receipt: SupportIntakeReceipt? = null, ) { @@ -1680,6 +1562,8 @@ class JvmSupportIntake( val cancellationPending: Boolean = false, val outcomeAmbiguous: Boolean = true, val latestUploadAttemptAtEpochMillis: Long? = null, + // Read descriptors written by early PR #386 builds, but never use wall time to confirm + // cancellation. Only the server's idempotency-key tombstone is terminal. val cancellationRequestedAtEpochMillis: Long? = null, val retryNotBeforeEpochMillis: Long? = null, val receipt: SupportIntakeReceipt? = null, @@ -1952,12 +1836,6 @@ private const val SUPPORT_RECOVERY_MAX_AGE_MILLIS = 30L * 24L * 60L * 60L * 1_00 private const val SUPPORT_SERVER_RETENTION_MAX_AGE_MILLIS = 30L * 24L * 60L * 60L * 1_000L private const val SUPPORT_RECEIPT_CLOCK_SKEW_MILLIS = 5L * 60L * 1_000L private const val SUPPORT_DESCRIPTOR_DELETE_RETRY_MILLIS = 60L * 1_000L -// The support service accepts request bodies synchronously with a 30-second read timeout. Wait -// beyond that bound before treating repeated 404 reconciliation responses as confirmed absence. -private const val SUPPORT_CANCELLATION_RECONCILE_WINDOW_MILLIS = 35L * 1_000L -private const val SUPPORT_CANCELLATION_RECONCILE_POLL_MILLIS = 500L -private const val MAX_CANCELLATION_RECONCILE_WINDOW_MILLIS = 60L * 1_000L -private const val NANOS_PER_MILLISECOND = 1_000_000L private const val SUPPORT_PENDING_RESTORE_MESSAGE = "Private support report recovery is temporarily unavailable. The app will retry automatically." private const val SUPPORT_COMPLETED_RESTORE_MESSAGE = From b3ba194bf2ade14761bbf6502bdbc53aba547894 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sat, 15 Aug 2026 03:11:35 +0200 Subject: [PATCH 05/14] fix(support): persist cancellation before tombstone --- .../app/JvmSupportIntakeTest.kt | 59 +++++++++++++++---- .../nextcloudnative/app/JvmSupportIntake.kt | 11 +++- 2 files changed, 58 insertions(+), 12 deletions(-) diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt index 6af9af3e8..cf0393846 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt @@ -16,6 +16,7 @@ import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertIs +import kotlin.test.assertNull import kotlin.test.assertTrue import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers @@ -643,32 +644,30 @@ class JvmSupportIntakeTest { @Test fun cancellationStopsTheActiveCallWhenIntentPersistenceFails() = runBlocking { - var directorySyncs = 0 + var rejectCancellationWrites = false testFixture( directorySync = { - directorySyncs += 1 - if (directorySyncs == 4) throw IOException("Synthetic cancellation persistence failure.") + if (rejectCancellationWrites) throw IOException("Synthetic cancellation persistence failure.") }, ).use { fixture -> fixture.server.enqueue( receiptResponse(fixture.statusUrl).newBuilder().headersDelay(10, TimeUnit.SECONDS).build(), ) - fixture.server.enqueue(MockResponse.Builder().code(204).build()) val submission = launch(Dispatchers.Default) { fixture.intake.submit("A refresh failed.", "nightly", emptyList()) } val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) assertEquals("POST", upload.method) + rejectCancellationWrites = true assertFalse(fixture.intake.cancel()) withTimeout(5_000) { submission.join() } - val cancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) - assertEquals("DELETE", cancellation.method) - assertEquals("/api/v1/receipts", cancellation.url.encodedPath) - assertEquals(upload.headers["Idempotency-Key"], cancellation.headers["Idempotency-Key"]) - assertIs(fixture.intake.states().value) - assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) + val retryable = assertIs(fixture.intake.states().value) + assertTrue(retryable.message.contains("was not sent")) + assertEquals(1, fixture.server.requestCount) + assertNull(fixture.server.takeRequest(200, TimeUnit.MILLISECONDS)) + assertTrue(File(fixture.temporaryRoot, "pending.json").isFile) } Unit } @@ -1078,6 +1077,46 @@ class JvmSupportIntakeTest { } } + @Test + fun restoredAmbiguousSubmissionAcceptsAuthoritativeCancellationTombstone() = runBlocking { + testFixture().use { fixture -> + fixture.server.enqueue( + receiptResponse(fixture.statusUrl).newBuilder().headersDelay(10, TimeUnit.SECONDS).build(), + ) + fixture.server.enqueue(MockResponse.Builder().onResponseStart(SocketEffect.CloseSocket()).build()) + + val submission = launch(Dispatchers.Default) { + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + } + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertTrue(fixture.intake.cancel()) + submission.join() + requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + fixture.intake.close() + + val descriptor = File(fixture.temporaryRoot, "pending.json") + descriptor.writeText( + descriptor.readText().replace( + "\"cancellationPending\":true", + "\"cancellationPending\":false", + ), + ) + val restored = fixture.newIntake() + assertIs(restored.states().value) + fixture.server.enqueue(MockResponse.Builder().code(410).build()) + + restored.retry() + + assertIs(restored.states().value) + val reconciliation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("GET", reconciliation.method) + assertEquals("/api/v1/receipts", reconciliation.url.encodedPath) + assertEquals(upload.headers["Idempotency-Key"], reconciliation.headers["Idempotency-Key"]) + assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) + restored.close() + } + } + @Test fun cancellationRetainsRecoveryUntilTerminalNoContent() = runBlocking { testFixture().use { fixture -> diff --git a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt index 973165fa6..bb746920a 100644 --- a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt +++ b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt @@ -736,7 +736,7 @@ class JvmSupportIntake( ambiguous = false, retryNotBeforeEpochMillis = response.retryNotBeforeEpochMillis(), ) - response.code == 410 && cancellationRequested.get() -> finishCancelled(submission) + response.code == 410 -> finishCancelled(submission) response.code in 400..499 -> finishRejected(submission, decodeProblem(responseText)) else -> retainForRetry( submission, @@ -822,6 +822,7 @@ class JvmSupportIntake( } responseCode == 404 -> retainForRetry(submission, "The upload did not complete. You can retry it safely.", false) + responseCode == 410 -> finishCancelled(submission) else -> retainForRetry( submission, "The upload result is uncertain. Check your connection before retrying.", @@ -857,7 +858,13 @@ class JvmSupportIntake( submission.cancellationPending = true submission.outcomeAmbiguous = true submission.receipt = receipt - persistPendingSafely(submission) + if (!persistPendingSafely(submission)) { + publishState(SupportDiagnosticsSubmissionState.RetryableFailure( + "Cancellation was not sent because its recovery state could not be stored safely. Keep the app open and retry.", + outcomeAmbiguous = true, + )) + return + } val request = Request.Builder() .url(baseUrl.newBuilder().addPathSegments("api/v1/receipts").build()) .header("Accept", "application/json") From c83c74f858916c317b93b9c699e41d11df3ba4d7 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sat, 15 Aug 2026 03:23:20 +0200 Subject: [PATCH 06/14] fix(support): complete cancellation during reconciliation --- .../app/JvmSupportIntakeTest.kt | 29 +++++++++++++++++++ .../nextcloudnative/app/JvmSupportIntake.kt | 18 +++++++----- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt index cf0393846..2d82c378f 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt @@ -992,6 +992,35 @@ class JvmSupportIntakeTest { } } + @Test + fun cancellationDuringReceiptReconciliationSendsAuthoritativeTombstone() = runBlocking { + testFixture().use { fixture -> + fixture.server.enqueue(MockResponse.Builder().onResponseStart(SocketEffect.CloseSocket()).build()) + fixture.server.enqueue( + MockResponse.Builder().code(404).headersDelay(10, TimeUnit.SECONDS).build(), + ) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) + + val submission = launch(Dispatchers.Default) { + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + } + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + val reconciliation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("GET", reconciliation.method) + + assertTrue(fixture.intake.cancel()) + submission.join() + + assertIs(fixture.intake.states().value) + val cancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", cancellation.method) + assertEquals("/api/v1/receipts", cancellation.url.encodedPath) + assertEquals(upload.headers["Idempotency-Key"], cancellation.headers["Idempotency-Key"]) + assertEquals(3, fixture.server.requestCount) + assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) + } + } + @Test fun serverFailureRemainsAmbiguousUntilCancellationIsConfirmed() = runBlocking { testFixture().use { fixture -> diff --git a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt index bb746920a..5d932b9c8 100644 --- a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt +++ b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt @@ -798,13 +798,17 @@ class JvmSupportIntake( response.code to response.readBoundedText() } } catch (_: IOException) { - retainForRetry( - submission, - uploadFailure.message?.filterSupportMetadata(MAX_SUPPORT_INTAKE_MESSAGE_LENGTH) - ?.takeIf(String::isNotBlank) - ?: "The upload result is uncertain. Check your connection before retrying.", - true, - ) + if (cancellationRequested.get() || submission.cancellationPending) { + cancelPendingSubmission(submission) + } else { + retainForRetry( + submission, + uploadFailure.message?.filterSupportMetadata(MAX_SUPPORT_INTAKE_MESSAGE_LENGTH) + ?.takeIf(String::isNotBlank) + ?: "The upload result is uncertain. Check your connection before retrying.", + true, + ) + } return } finally { activeCall.compareAndSet(call, null) From 8e282f6adbc1961086c86822b7bb58ef5fe0aadd Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sat, 15 Aug 2026 03:35:57 +0200 Subject: [PATCH 07/14] fix(support): honor cancellation after receipt lookup --- .../app/JvmSupportIntakeTest.kt | 40 +++++++++++++++++++ .../nextcloudnative/app/JvmSupportIntake.kt | 6 +++ 2 files changed, 46 insertions(+) diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt index 2d82c378f..9ae0b52cb 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt @@ -1021,6 +1021,42 @@ class JvmSupportIntakeTest { } } + @Test + fun cancellationAfterReceiptLookupCompletesSendsAuthoritativeTombstone() = runBlocking { + val lookupCompleted = CountDownLatch(1) + val allowLookupResult = CountDownLatch(1) + testFixture( + afterReceiptLookup = { + lookupCompleted.countDown() + assertTrue(allowLookupResult.await(2, TimeUnit.SECONDS)) + }, + ).use { fixture -> + fixture.server.enqueue(MockResponse.Builder().onResponseStart(SocketEffect.CloseSocket()).build()) + fixture.server.enqueue(MockResponse.Builder().code(404).build()) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) + + val submission = launch(Dispatchers.Default) { + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + } + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + val reconciliation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("GET", reconciliation.method) + assertTrue(lookupCompleted.await(2, TimeUnit.SECONDS)) + + assertTrue(fixture.intake.cancel()) + allowLookupResult.countDown() + submission.join() + + assertIs(fixture.intake.states().value) + val cancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", cancellation.method) + assertEquals("/api/v1/receipts", cancellation.url.encodedPath) + assertEquals(upload.headers["Idempotency-Key"], cancellation.headers["Idempotency-Key"]) + assertEquals(3, fixture.server.requestCount) + assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) + } + } + @Test fun serverFailureRemainsAmbiguousUntilCancellationIsConfirmed() = runBlocking { testFixture().use { fixture -> @@ -1732,6 +1768,7 @@ class JvmSupportIntakeTest { beforeSubmissionPreparation: () -> Unit = {}, beforeBundlePackaging: () -> Unit = {}, afterBundlePackaging: () -> Unit = {}, + afterReceiptLookup: () -> Unit = {}, privateFileDelete: (File) -> Boolean = File::delete, pendingDescriptorRead: (File) -> String = { descriptor -> descriptor.readText() }, completedDescriptorRead: (File) -> String = { descriptor -> descriptor.readText() }, @@ -1796,6 +1833,7 @@ class JvmSupportIntakeTest { beforeSubmissionPreparation = beforeSubmissionPreparation, beforeBundlePackaging = beforeBundlePackaging, afterBundlePackaging = afterBundlePackaging, + afterReceiptLookup = afterReceiptLookup, privateFileDelete = privateFileDelete, pendingDescriptorRead = pendingDescriptorRead, completedDescriptorRead = completedDescriptorRead, @@ -1840,6 +1878,7 @@ class JvmSupportIntakeTest { val beforeSubmissionPreparation: () -> Unit, val beforeBundlePackaging: () -> Unit, val afterBundlePackaging: () -> Unit, + val afterReceiptLookup: () -> Unit, val privateFileDelete: (File) -> Boolean, val pendingDescriptorRead: (File) -> String, val completedDescriptorRead: (File) -> String, @@ -1863,6 +1902,7 @@ class JvmSupportIntakeTest { beforeSubmissionPreparation = beforeSubmissionPreparation, beforeBundlePackaging = beforeBundlePackaging, afterBundlePackaging = afterBundlePackaging, + afterReceiptLookup = afterReceiptLookup, privateFileDelete = privateFileDelete, pendingDescriptorRead = pendingDescriptorRead, completedDescriptorRead = completedDescriptorRead, diff --git a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt index 5d932b9c8..944050d28 100644 --- a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt +++ b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt @@ -66,6 +66,7 @@ class JvmSupportIntake( private val beforeSubmissionPreparation: () -> Unit = {}, private val beforeBundlePackaging: () -> Unit = {}, private val afterBundlePackaging: () -> Unit = {}, + private val afterReceiptLookup: () -> Unit = {}, private val privateFileDelete: (File) -> Boolean = File::delete, private val pendingDescriptorRead: (File) -> String = { descriptor -> descriptor.readText(Charsets.UTF_8) @@ -813,6 +814,11 @@ class JvmSupportIntake( } finally { activeCall.compareAndSet(call, null) } + afterReceiptLookup() + if (cancellationRequested.get() || submission.cancellationPending) { + cancelPendingSubmission(submission) + return + } val (responseCode, responseText) = responseResult when { responseCode in 200..299 -> { From 7f30516299bde67577320dea00b7693dcfbf3295 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sat, 15 Aug 2026 03:52:15 +0200 Subject: [PATCH 08/14] fix(support): preserve cancellation across response races --- .../app/JvmSupportIntakeTest.kt | 103 ++++++++++++++++++ .../nextcloudnative/app/JvmSupportIntake.kt | 16 ++- 2 files changed, 114 insertions(+), 5 deletions(-) diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt index 9ae0b52cb..da370a974 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt @@ -1057,6 +1057,65 @@ class JvmSupportIntakeTest { } } + @Test + fun cancellationAfterUploadResponseCompletesSendsAuthoritativeTombstone() = runBlocking { + val responseCompleted = CountDownLatch(1) + val allowResponseResult = CountDownLatch(1) + testFixture( + afterUploadResponse = { + responseCompleted.countDown() + assertTrue(allowResponseResult.await(2, TimeUnit.SECONDS)) + }, + ).use { fixture -> + fixture.server.enqueue(MockResponse.Builder().code(503).build()) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) + + val submission = launch(Dispatchers.Default) { + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + } + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertTrue(responseCompleted.await(2, TimeUnit.SECONDS)) + + assertTrue(fixture.intake.cancel()) + allowResponseResult.countDown() + submission.join() + + assertIs(fixture.intake.states().value) + val cancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", cancellation.method) + assertEquals("/api/v1/receipts", cancellation.url.encodedPath) + assertEquals(upload.headers["Idempotency-Key"], cancellation.headers["Idempotency-Key"]) + assertEquals(2, fixture.server.requestCount) + assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) + } + } + + @Test + fun cancellationAfterReconciledReceiptAbsenceUsesAuthoritativeTombstone() = runBlocking { + testFixture().use { fixture -> + fixture.server.enqueue(MockResponse.Builder().onResponseStart(SocketEffect.CloseSocket()).build()) + fixture.server.enqueue(MockResponse.Builder().code(404).build()) + + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + + assertIs(fixture.intake.states().value) + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + val reconciliation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("GET", reconciliation.method) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) + + assertTrue(fixture.intake.cancel()) + + assertIs(fixture.intake.states().value) + val cancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", cancellation.method) + assertEquals("/api/v1/receipts", cancellation.url.encodedPath) + assertEquals(upload.headers["Idempotency-Key"], cancellation.headers["Idempotency-Key"]) + assertEquals(3, fixture.server.requestCount) + assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) + } + } + @Test fun serverFailureRemainsAmbiguousUntilCancellationIsConfirmed() = runBlocking { testFixture().use { fixture -> @@ -1532,6 +1591,46 @@ class JvmSupportIntakeTest { } } + @Test + fun reconcilesRestoredCancellationBeforeApplyingRecoveryExpiry() = runBlocking { + testFixture().use { fixture -> + fixture.server.enqueue(receiptResponse(fixture.statusUrl).newBuilder().headersDelay(10, TimeUnit.SECONDS).build()) + fixture.server.enqueue(MockResponse.Builder().code(503).build()) + + val submission = launch(Dispatchers.Default) { + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + } + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertTrue(fixture.intake.cancel()) + submission.join() + assertIs(fixture.intake.states().value) + requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + fixture.intake.close() + + val descriptor = File(fixture.temporaryRoot, "pending.json") + val expiredCreatedAt = Instant.now().minus(31, ChronoUnit.DAYS).toEpochMilli() + descriptor.writeText( + descriptor.readText().replace( + Regex("\"createdAtEpochMillis\":\\d+"), + "\"createdAtEpochMillis\":$expiredCreatedAt", + ), + ) + val restored = fixture.newIntake() + assertIs(restored.states().value) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) + + restored.retry() + + assertIs(restored.states().value) + val cancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", cancellation.method) + assertEquals("/api/v1/receipts", cancellation.url.encodedPath) + assertEquals(upload.headers["Idempotency-Key"], cancellation.headers["Idempotency-Key"]) + assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) + restored.close() + } + } + @Test fun restoresCancellationKeyWhenTheWallClockMovesBackward() = runBlocking { testFixture().use { fixture -> @@ -1768,6 +1867,7 @@ class JvmSupportIntakeTest { beforeSubmissionPreparation: () -> Unit = {}, beforeBundlePackaging: () -> Unit = {}, afterBundlePackaging: () -> Unit = {}, + afterUploadResponse: () -> Unit = {}, afterReceiptLookup: () -> Unit = {}, privateFileDelete: (File) -> Boolean = File::delete, pendingDescriptorRead: (File) -> String = { descriptor -> descriptor.readText() }, @@ -1833,6 +1933,7 @@ class JvmSupportIntakeTest { beforeSubmissionPreparation = beforeSubmissionPreparation, beforeBundlePackaging = beforeBundlePackaging, afterBundlePackaging = afterBundlePackaging, + afterUploadResponse = afterUploadResponse, afterReceiptLookup = afterReceiptLookup, privateFileDelete = privateFileDelete, pendingDescriptorRead = pendingDescriptorRead, @@ -1878,6 +1979,7 @@ class JvmSupportIntakeTest { val beforeSubmissionPreparation: () -> Unit, val beforeBundlePackaging: () -> Unit, val afterBundlePackaging: () -> Unit, + val afterUploadResponse: () -> Unit, val afterReceiptLookup: () -> Unit, val privateFileDelete: (File) -> Boolean, val pendingDescriptorRead: (File) -> String, @@ -1902,6 +2004,7 @@ class JvmSupportIntakeTest { beforeSubmissionPreparation = beforeSubmissionPreparation, beforeBundlePackaging = beforeBundlePackaging, afterBundlePackaging = afterBundlePackaging, + afterUploadResponse = afterUploadResponse, afterReceiptLookup = afterReceiptLookup, privateFileDelete = privateFileDelete, pendingDescriptorRead = pendingDescriptorRead, diff --git a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt index 944050d28..90f45949b 100644 --- a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt +++ b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt @@ -66,6 +66,7 @@ class JvmSupportIntake( private val beforeSubmissionPreparation: () -> Unit = {}, private val beforeBundlePackaging: () -> Unit = {}, private val afterBundlePackaging: () -> Unit = {}, + private val afterUploadResponse: () -> Unit = {}, private val afterReceiptLookup: () -> Unit = {}, private val privateFileDelete: (File) -> Boolean = File::delete, private val pendingDescriptorRead: (File) -> String = { descriptor -> @@ -265,10 +266,6 @@ class JvmSupportIntake( if (!submission.belongsTo(synchronized(lock) { activeAccountIdentity })) { return@withContext } - if (submission.recoveryExpired(System.currentTimeMillis())) { - finishRejected(submission, "The private report recovery capability expired and was removed from this device.") - return@withContext - } if (submission.cancellationPending) { cancellationRequested.set(true) publishState( @@ -278,6 +275,10 @@ class JvmSupportIntake( cancelPendingSubmission(submission) return@withContext } + if (submission.recoveryExpired(System.currentTimeMillis())) { + finishRejected(submission, "The private report recovery capability expired and was removed from this device.") + return@withContext + } val waitMillis = submission.retryNotBeforeEpochMillis?.minus(System.currentTimeMillis()) ?: 0L if (waitMillis > 0L) { publishState(SupportDiagnosticsSubmissionState.RetryableFailure( @@ -725,6 +726,11 @@ class JvmSupportIntake( call.execute().use { response -> val responseText = response.readBoundedText() activeCall.compareAndSet(call, null) + afterUploadResponse() + if (cancellationRequested.get() || submission.cancellationPending) { + cancelPendingSubmission(submission) + return + } when { response.isSuccessful -> finishReceived(submission, decodeReceipt(responseText)) response.code == 408 -> reconcileAfterAmbiguousResult( @@ -831,7 +837,7 @@ class JvmSupportIntake( } } responseCode == 404 -> - retainForRetry(submission, "The upload did not complete. You can retry it safely.", false) + retainForRetry(submission, "The upload did not complete. You can retry it safely.", true) responseCode == 410 -> finishCancelled(submission) else -> retainForRetry( submission, From ca67ddee3f59389fcc22958f04ae086c39680e7b Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sat, 15 Aug 2026 03:55:26 +0200 Subject: [PATCH 09/14] fix(support): distinguish upload attempts from retry state --- .../nextcloudnative/app/JvmSupportIntake.kt | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt index 90f45949b..7e76fd5f9 100644 --- a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt +++ b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt @@ -392,7 +392,7 @@ class JvmSupportIntake( private fun cancelAfterIntentPublished(): Boolean { val submission = synchronized(lock) { pending } if (submission != null) { - if (!submission.outcomeAmbiguous && activeCall.get() == null) { + if (!submission.cancellationRequiresTombstone && activeCall.get() == null) { finishCancelled(submission) return true } @@ -670,6 +670,7 @@ class JvmSupportIntake( } submission.latestUploadAttemptAtEpochMillis = System.currentTimeMillis().coerceAtLeast(0L) submission.outcomeAmbiguous = true + submission.cancellationRequiresTombstone = true if (!persistPendingSafely(submission)) { finishRejected(submission, "The private support submission could not be retained safely on this device.") return @@ -837,7 +838,7 @@ class JvmSupportIntake( } } responseCode == 404 -> - retainForRetry(submission, "The upload did not complete. You can retry it safely.", true) + retainForRetry(submission, "The upload did not complete. You can retry it safely.", false) responseCode == 410 -> finishCancelled(submission) else -> retainForRetry( submission, @@ -1198,6 +1199,7 @@ class JvmSupportIntake( context = submission.context, cancellationPending = submission.cancellationPending, outcomeAmbiguous = submission.outcomeAmbiguous, + cancellationRequiresTombstone = submission.cancellationRequiresTombstone, latestUploadAttemptAtEpochMillis = submission.latestUploadAttemptAtEpochMillis, retryNotBeforeEpochMillis = submission.retryNotBeforeEpochMillis, receipt = submission.receipt, @@ -1324,7 +1326,7 @@ class JvmSupportIntake( } else { persisted.createdAtEpochMillis.saturatingAdd(SUPPORT_RECOVERY_MAX_AGE_MILLIS) } - require(nowEpochMillis <= recoveryDeadlineEpochMillis) + require(persisted.cancellationPending || nowEpochMillis <= recoveryDeadlineEpochMillis) val archiveAgeMillis = (nowEpochMillis - persisted.createdAtEpochMillis).coerceAtLeast(0L) val archiveIsRetained = archiveAgeMillis <= SUPPORT_TEMPORARY_MAX_AGE_MILLIS val archive = persisted.archiveName?.let { archiveName -> @@ -1357,6 +1359,12 @@ class JvmSupportIntake( context = persisted.context, cancellationPending = persisted.cancellationPending, outcomeAmbiguous = persisted.outcomeAmbiguous, + cancellationRequiresTombstone = persisted.cancellationRequiresTombstone + ?: ( + persisted.latestUploadAttemptAtEpochMillis != null || + persisted.outcomeAmbiguous || + persisted.receipt != null + ), latestUploadAttemptAtEpochMillis = persisted.latestUploadAttemptAtEpochMillis, retryNotBeforeEpochMillis = retryNotBeforeEpochMillis, receipt = persisted.receipt, @@ -1544,6 +1552,7 @@ class JvmSupportIntake( val context: PreparedSupportSubmissionContext, var cancellationPending: Boolean = false, var outcomeAmbiguous: Boolean = false, + var cancellationRequiresTombstone: Boolean = false, var latestUploadAttemptAtEpochMillis: Long? = null, var retryNotBeforeEpochMillis: Long? = null, var receipt: SupportIntakeReceipt? = null, @@ -1584,6 +1593,7 @@ class JvmSupportIntake( val context: PreparedSupportSubmissionContext, val cancellationPending: Boolean = false, val outcomeAmbiguous: Boolean = true, + val cancellationRequiresTombstone: Boolean? = null, val latestUploadAttemptAtEpochMillis: Long? = null, // Read descriptors written by early PR #386 builds, but never use wall time to confirm // cancellation. Only the server's idempotency-key tombstone is terminal. From 852a500f6cd99a8501fd3ae070af0cdf6d3fd2cd Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sat, 15 Aug 2026 04:09:13 +0200 Subject: [PATCH 10/14] fix(support): minimize cancellation recovery state --- .../app/JvmSupportIntakeTest.kt | 51 ++++++++++++++++++ .../nextcloudnative/app/JvmSupportIntake.kt | 52 +++++++++++++++---- 2 files changed, 93 insertions(+), 10 deletions(-) diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt index da370a974..96da3c731 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt @@ -559,6 +559,29 @@ class JvmSupportIntakeTest { } } + @Test + fun transportGateClosingBeforePostStillAllowsLocalCancellation() = runBlocking { + var gateChecks = 0 + testFixture( + supportMutationsAllowed = { + gateChecks += 1 + gateChecks <= 2 + }, + ).use { fixture -> + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + + assertIs(fixture.intake.states().value) + assertEquals(0, fixture.server.requestCount) + assertTrue(File(fixture.temporaryRoot, "pending.json").isFile) + + assertTrue(fixture.intake.cancel()) + + assertIs(fixture.intake.states().value) + assertEquals(0, fixture.server.requestCount) + assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) + } + } + @Test fun cancellationWinsBeforeTheUploadCallIsRegistered() = runBlocking { val registrationEntered = CountDownLatch(1) @@ -1551,6 +1574,34 @@ class JvmSupportIntakeTest { } } + @Test + fun failedCancellationRetainsOnlyMinimalRecoveryState() = runBlocking { + testFixture().use { fixture -> + fixture.server.enqueue(MockResponse.Builder().code(503).build()) + fixture.intake.submit( + "Private cancellation reproduction note.", + "nightly", + listOf(SupportDiagnosticFieldDraft("private_field", "Private value")), + ) + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + fixture.server.enqueue(MockResponse.Builder().code(503).build()) + + assertTrue(fixture.intake.cancel()) + + assertIs(fixture.intake.states().value) + val cancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", cancellation.method) + assertEquals(upload.headers["Idempotency-Key"], cancellation.headers["Idempotency-Key"]) + assertFalse(fixture.temporaryRoot.listFiles().orEmpty().any { it.extension == "zip" }) + val descriptor = File(fixture.temporaryRoot, "pending.json").readText() + assertTrue(descriptor.contains(upload.headers["Idempotency-Key"].orEmpty())) + assertTrue(descriptor.contains("\"cancellationPending\":true")) + assertFalse(descriptor.contains("Private cancellation reproduction note.")) + assertFalse(descriptor.contains("private_field")) + assertFalse(descriptor.contains("Private value")) + } + } + @Test fun keepsCancellationKeyAfterTheLocalArchiveRetentionWindow() = runBlocking { testFixture().use { fixture -> diff --git a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt index 7e76fd5f9..bbc51e4ca 100644 --- a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt +++ b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt @@ -668,13 +668,6 @@ class JvmSupportIntake( retainForRetry(submission, READ_ONLY_SUPPORT_MESSAGE, ambiguous = false) return } - submission.latestUploadAttemptAtEpochMillis = System.currentTimeMillis().coerceAtLeast(0L) - submission.outcomeAmbiguous = true - submission.cancellationRequiresTombstone = true - if (!persistPendingSafely(submission)) { - finishRejected(submission, "The private support submission could not be retained safely on this device.") - return - } val archive = requireNotNull(submission.archive) { "The private support archive has not been prepared." } require(archive.isFile && archive.length() in 1L..MAX_SUPPORT_ARCHIVE_BYTES) val metadata = json.encodeToString(SupportIntakeMetadata.serializer(), submission.metadata) @@ -708,6 +701,13 @@ class JvmSupportIntake( retainForRetry(submission, READ_ONLY_SUPPORT_MESSAGE, ambiguous = false) return } + submission.latestUploadAttemptAtEpochMillis = System.currentTimeMillis().coerceAtLeast(0L) + submission.outcomeAmbiguous = true + submission.cancellationRequiresTombstone = true + if (!persistPendingSafely(submission)) { + finishRejected(submission, "The private support submission could not be retained safely on this device.") + return + } publishState(SupportDiagnosticsSubmissionState.Uploading(0f)) val call = client.newCall(request) beforeCallRegistration() @@ -875,7 +875,7 @@ class JvmSupportIntake( submission.cancellationPending = true submission.outcomeAmbiguous = true submission.receipt = receipt - if (!persistPendingSafely(submission)) { + if (!persistMinimalCancellationSafely(submission)) { publishState(SupportDiagnosticsSubmissionState.RetryableFailure( "Cancellation was not sent because its recovery state could not be stored safely. Keep the app open and retry.", outcomeAmbiguous = true, @@ -922,6 +922,38 @@ class JvmSupportIntake( } } + private fun persistMinimalCancellationSafely(submission: PendingSubmission): Boolean { + val archive = submission.archive + val metadata = submission.metadata + val context = submission.context + val receipt = submission.receipt + val retryNotBeforeEpochMillis = submission.retryNotBeforeEpochMillis + submission.archive = null + submission.metadata = SupportIntakeMetadata( + title = "", + description = "", + release = SupportIntakeRelease("", "", "", "", ""), + ) + submission.context = PreparedSupportSubmissionContext( + sanitizedReproductionSteps = null, + featureState = emptyList(), + confirmedAtEpochMillis = 0L, + events = emptyList(), + ) + submission.receipt = null + submission.retryNotBeforeEpochMillis = null + if (!persistPendingSafely(submission)) { + submission.archive = archive + submission.metadata = metadata + submission.context = context + submission.receipt = receipt + submission.retryNotBeforeEpochMillis = retryNotBeforeEpochMillis + return false + } + deletePrivateFileOrRetry(archive) + return true + } + private fun finishSubmitted(submission: PendingSubmission, receipt: SupportIntakeReceipt) { validateReceipt(receipt) val existingCompletion = synchronized(lock) { @@ -1545,11 +1577,11 @@ class JvmSupportIntake( private data class PendingSubmission( var archive: File?, - val metadata: SupportIntakeMetadata, + var metadata: SupportIntakeMetadata, val idempotencyKey: String, val createdAtEpochMillis: Long, val originAccountIdentity: String, - val context: PreparedSupportSubmissionContext, + var context: PreparedSupportSubmissionContext, var cancellationPending: Boolean = false, var outcomeAmbiguous: Boolean = false, var cancellationRequiresTombstone: Boolean = false, From d7f5afd847a02830ee2e50d13aad2a5fbdcf2caf Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sat, 15 Aug 2026 04:22:23 +0200 Subject: [PATCH 11/14] fix(support): minimize cancellation intent atomically --- .../app/JvmSupportIntakeTest.kt | 42 ++++++++++++++++ .../nextcloudnative/app/JvmSupportIntake.kt | 48 +++++++++++++------ 2 files changed, 76 insertions(+), 14 deletions(-) diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt index 96da3c731..d8f846826 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt @@ -1602,6 +1602,48 @@ class JvmSupportIntakeTest { } } + @Test + fun restorationMinimizesPreviouslyPersistedCancellationIntent() = runBlocking { + testFixture().use { fixture -> + fixture.server.enqueue(MockResponse.Builder().code(503).build()) + fixture.intake.submit( + "Private restored cancellation note.", + "nightly", + listOf(SupportDiagnosticFieldDraft("private_restored_field", "Private restored value")), + ) + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + val descriptor = File(fixture.temporaryRoot, "pending.json") + assertTrue(descriptor.readText().contains("Private restored cancellation note.")) + assertTrue(fixture.temporaryRoot.listFiles().orEmpty().any { it.extension == "zip" }) + fixture.intake.close() + descriptor.writeText( + descriptor.readText().replace( + "\"cancellationPending\":false", + "\"cancellationPending\":true", + ), + ) + + val restored = fixture.newIntake() + + assertIs(restored.states().value) + assertFalse(fixture.temporaryRoot.listFiles().orEmpty().any { it.extension == "zip" }) + val minimized = descriptor.readText() + assertTrue(minimized.contains(upload.headers["Idempotency-Key"].orEmpty())) + assertTrue(minimized.contains("\"cancellationPending\":true")) + assertFalse(minimized.contains("Private restored cancellation note.")) + assertFalse(minimized.contains("private_restored_field")) + assertFalse(minimized.contains("Private restored value")) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) + + restored.retry() + + assertIs(restored.states().value) + assertEquals("DELETE", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) + assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) + restored.close() + } + } + @Test fun keepsCancellationKeyAfterTheLocalArchiveRetentionWindow() = runBlocking { testFixture().use { fixture -> diff --git a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt index bbc51e4ca..a68ac9705 100644 --- a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt +++ b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt @@ -398,7 +398,11 @@ class JvmSupportIntake( } submission.cancellationPending = true submission.outcomeAmbiguous = true - val cancellationPersisted = persistPendingSafely(submission) + val archive = submission.archive + val cancellationPersisted = persistMinimalCancellationSafely( + submission, + deleteArchiveAfterPersist = false, + ) val call = activeCall.getAndSet(null) if (!cancellationPersisted) { call?.cancel() @@ -408,12 +412,13 @@ class JvmSupportIntake( )) return false } + call?.cancel() + deletePrivateFileOrRetry(archive) if (call != null) { publishState( SupportDiagnosticsSubmissionState.Cancelling, submission.originAccountIdentity, ) - call.cancel() return true } } @@ -922,12 +927,30 @@ class JvmSupportIntake( } } - private fun persistMinimalCancellationSafely(submission: PendingSubmission): Boolean { + private fun persistMinimalCancellationSafely( + submission: PendingSubmission, + deleteArchiveAfterPersist: Boolean = true, + ): Boolean { val archive = submission.archive val metadata = submission.metadata val context = submission.context val receipt = submission.receipt val retryNotBeforeEpochMillis = submission.retryNotBeforeEpochMillis + stripPrivateCancellationPayload(submission) + if (!persistPendingSafely(submission)) { + submission.archive = archive + submission.metadata = metadata + submission.context = context + submission.receipt = receipt + submission.retryNotBeforeEpochMillis = retryNotBeforeEpochMillis + return false + } + if (deleteArchiveAfterPersist) deletePrivateFileOrRetry(archive) + return true + } + + private fun stripPrivateCancellationPayload(submission: PendingSubmission): File? { + val archive = submission.archive submission.archive = null submission.metadata = SupportIntakeMetadata( title = "", @@ -942,16 +965,7 @@ class JvmSupportIntake( ) submission.receipt = null submission.retryNotBeforeEpochMillis = null - if (!persistPendingSafely(submission)) { - submission.archive = archive - submission.metadata = metadata - submission.context = context - submission.receipt = receipt - submission.retryNotBeforeEpochMillis = retryNotBeforeEpochMillis - return false - } - deletePrivateFileOrRetry(archive) - return true + return archive } private fun finishSubmitted(submission: PendingSubmission, receipt: SupportIntakeReceipt) { @@ -1382,7 +1396,7 @@ class JvmSupportIntake( } } pendingDescriptorRestorePending.set(false) - PendingSubmission( + val restored = PendingSubmission( archive = archive, metadata = persisted.metadata, idempotencyKey = persisted.idempotencyKey, @@ -1401,6 +1415,12 @@ class JvmSupportIntake( retryNotBeforeEpochMillis = retryNotBeforeEpochMillis, receipt = persisted.receipt, ) + if (restored.cancellationPending) { + val privateArchive = stripPrivateCancellationPayload(restored) + persistPending(restored) + deletePrivateFileOrRetry(privateArchive) + } + restored } catch (failure: Throwable) { if (failure is IOException || failure is SecurityException) { val retryWasNotScheduled = pendingDescriptorRestorePending.compareAndSet(false, true) From 09cf4e77728f9601b80bf8a5f43f715c62bad803 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sat, 15 Aug 2026 04:37:59 +0200 Subject: [PATCH 12/14] fix(support): serialize cancellation disposition --- .../app/JvmSupportIntakeTest.kt | 74 +++++++++++++++++++ .../nextcloudnative/app/JvmSupportIntake.kt | 45 +++++++++-- 2 files changed, 114 insertions(+), 5 deletions(-) diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt index d8f846826..b9ad9bb57 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt @@ -1080,6 +1080,40 @@ class JvmSupportIntakeTest { } } + @Test + fun cancellationAfterReceiptIntentCheckContinuesWithAuthoritativeTombstone() = runBlocking { + val dispositionEntered = CountDownLatch(1) + val allowDisposition = CountDownLatch(1) + testFixture( + beforeReceiptResponseDisposition = { + dispositionEntered.countDown() + assertTrue(allowDisposition.await(2, TimeUnit.SECONDS)) + }, + ).use { fixture -> + fixture.server.enqueue(MockResponse.Builder().onResponseStart(SocketEffect.CloseSocket()).build()) + fixture.server.enqueue(MockResponse.Builder().code(404).build()) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) + + val submission = launch(Dispatchers.Default) { + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + } + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("GET", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) + assertTrue(dispositionEntered.await(2, TimeUnit.SECONDS)) + + assertTrue(fixture.intake.cancel()) + allowDisposition.countDown() + submission.join() + + assertIs(fixture.intake.states().value) + val cancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", cancellation.method) + assertEquals("/api/v1/receipts", cancellation.url.encodedPath) + assertEquals(upload.headers["Idempotency-Key"], cancellation.headers["Idempotency-Key"]) + assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) + } + } + @Test fun cancellationAfterUploadResponseCompletesSendsAuthoritativeTombstone() = runBlocking { val responseCompleted = CountDownLatch(1) @@ -1113,6 +1147,38 @@ class JvmSupportIntakeTest { } } + @Test + fun cancellationAfterUploadIntentCheckContinuesWithAuthoritativeTombstone() = runBlocking { + val dispositionEntered = CountDownLatch(1) + val allowDisposition = CountDownLatch(1) + testFixture( + beforeUploadResponseDisposition = { + dispositionEntered.countDown() + assertTrue(allowDisposition.await(2, TimeUnit.SECONDS)) + }, + ).use { fixture -> + fixture.server.enqueue(MockResponse.Builder().code(503).build()) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) + + val submission = launch(Dispatchers.Default) { + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + } + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertTrue(dispositionEntered.await(2, TimeUnit.SECONDS)) + + assertTrue(fixture.intake.cancel()) + allowDisposition.countDown() + submission.join() + + assertIs(fixture.intake.states().value) + val cancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", cancellation.method) + assertEquals("/api/v1/receipts", cancellation.url.encodedPath) + assertEquals(upload.headers["Idempotency-Key"], cancellation.headers["Idempotency-Key"]) + assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) + } + } + @Test fun cancellationAfterReconciledReceiptAbsenceUsesAuthoritativeTombstone() = runBlocking { testFixture().use { fixture -> @@ -1961,7 +2027,9 @@ class JvmSupportIntakeTest { beforeBundlePackaging: () -> Unit = {}, afterBundlePackaging: () -> Unit = {}, afterUploadResponse: () -> Unit = {}, + beforeUploadResponseDisposition: () -> Unit = {}, afterReceiptLookup: () -> Unit = {}, + beforeReceiptResponseDisposition: () -> Unit = {}, privateFileDelete: (File) -> Boolean = File::delete, pendingDescriptorRead: (File) -> String = { descriptor -> descriptor.readText() }, completedDescriptorRead: (File) -> String = { descriptor -> descriptor.readText() }, @@ -2027,7 +2095,9 @@ class JvmSupportIntakeTest { beforeBundlePackaging = beforeBundlePackaging, afterBundlePackaging = afterBundlePackaging, afterUploadResponse = afterUploadResponse, + beforeUploadResponseDisposition = beforeUploadResponseDisposition, afterReceiptLookup = afterReceiptLookup, + beforeReceiptResponseDisposition = beforeReceiptResponseDisposition, privateFileDelete = privateFileDelete, pendingDescriptorRead = pendingDescriptorRead, completedDescriptorRead = completedDescriptorRead, @@ -2073,7 +2143,9 @@ class JvmSupportIntakeTest { val beforeBundlePackaging: () -> Unit, val afterBundlePackaging: () -> Unit, val afterUploadResponse: () -> Unit, + val beforeUploadResponseDisposition: () -> Unit, val afterReceiptLookup: () -> Unit, + val beforeReceiptResponseDisposition: () -> Unit, val privateFileDelete: (File) -> Boolean, val pendingDescriptorRead: (File) -> String, val completedDescriptorRead: (File) -> String, @@ -2098,7 +2170,9 @@ class JvmSupportIntakeTest { beforeBundlePackaging = beforeBundlePackaging, afterBundlePackaging = afterBundlePackaging, afterUploadResponse = afterUploadResponse, + beforeUploadResponseDisposition = beforeUploadResponseDisposition, afterReceiptLookup = afterReceiptLookup, + beforeReceiptResponseDisposition = beforeReceiptResponseDisposition, privateFileDelete = privateFileDelete, pendingDescriptorRead = pendingDescriptorRead, completedDescriptorRead = completedDescriptorRead, diff --git a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt index a68ac9705..a93c68d41 100644 --- a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt +++ b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt @@ -67,7 +67,9 @@ class JvmSupportIntake( private val beforeBundlePackaging: () -> Unit = {}, private val afterBundlePackaging: () -> Unit = {}, private val afterUploadResponse: () -> Unit = {}, + private val beforeUploadResponseDisposition: () -> Unit = {}, private val afterReceiptLookup: () -> Unit = {}, + private val beforeReceiptResponseDisposition: () -> Unit = {}, private val privateFileDelete: (File) -> Boolean = File::delete, private val pendingDescriptorRead: (File) -> String = { descriptor -> descriptor.readText(Charsets.UTF_8) @@ -94,6 +96,7 @@ class JvmSupportIntake( private val cancellationRequested = AtomicBoolean(false) private val shutdownRequested = AtomicBoolean(false) private val operationActive = AtomicBoolean(false) + private val cancellationContinuationRequested = AtomicBoolean(false) private val rejectedPendingDescriptorCleanup = AtomicBoolean(false) private val pendingDescriptorRestorePending = AtomicBoolean(false) private val completedDescriptorRestorePending = AtomicBoolean(false) @@ -318,6 +321,7 @@ class JvmSupportIntake( private fun supportMutationsAreAllowed(): Boolean = runCatching(supportMutationsAllowed).getOrDefault(false) private fun endOperation() { + var cancellationContinuation: PendingSubmission? = null synchronized(lock) { if (actualState is SupportDiagnosticsSubmissionState.Cancelling && pending == null) { publishStateLocked( @@ -326,7 +330,28 @@ class JvmSupportIntake( ) } operationActive.set(false) - refreshVisibleStateLocked() + val submission = pending + if ( + cancellationContinuationRequested.getAndSet(false) && + submission?.cancellationPending == true && + !shutdownRequested.get() && + operationActive.compareAndSet(false, true) + ) { + cancellationContinuation = submission + } else { + refreshVisibleStateLocked() + } + } + cancellationContinuation?.let { submission -> + try { + publishState( + SupportDiagnosticsSubmissionState.Cancelling, + submission.originAccountIdentity, + ) + cancelPendingSubmission(submission) + } finally { + endOperation() + } } } @@ -399,11 +424,12 @@ class JvmSupportIntake( submission.cancellationPending = true submission.outcomeAmbiguous = true val archive = submission.archive + val callAtIntent = activeCall.get() val cancellationPersisted = persistMinimalCancellationSafely( submission, deleteArchiveAfterPersist = false, ) - val call = activeCall.getAndSet(null) + val call = callAtIntent?.takeIf { activeCall.compareAndSet(it, null) } if (!cancellationPersisted) { call?.cancel() publishState(SupportDiagnosticsSubmissionState.RetryableFailure( @@ -421,9 +447,15 @@ class JvmSupportIntake( ) return true } - } - if (submission != null) { - if (beginOperation()) { + val startCancellation = synchronized(lock) { + if (operationActive.compareAndSet(false, true)) { + true + } else { + cancellationContinuationRequested.set(true) + false + } + } + if (startCancellation) { try { publishState( SupportDiagnosticsSubmissionState.Cancelling, @@ -737,6 +769,7 @@ class JvmSupportIntake( cancelPendingSubmission(submission) return } + beforeUploadResponseDisposition() when { response.isSuccessful -> finishReceived(submission, decodeReceipt(responseText)) response.code == 408 -> reconcileAfterAmbiguousResult( @@ -831,6 +864,7 @@ class JvmSupportIntake( cancelPendingSubmission(submission) return } + beforeReceiptResponseDisposition() val (responseCode, responseText) = responseResult when { responseCode in 200..299 -> { @@ -877,6 +911,7 @@ class JvmSupportIntake( submission: PendingSubmission, receipt: SupportIntakeReceipt? = submission.receipt, ) { + cancellationContinuationRequested.set(false) submission.cancellationPending = true submission.outcomeAmbiguous = true submission.receipt = receipt From a174511529efdedd8cd8574e51e6b66e8e61bc2c Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sat, 15 Aug 2026 04:52:46 +0200 Subject: [PATCH 13/14] fix(support): serialize cancellation minimization --- .../app/JvmSupportIntakeTest.kt | 53 +++++++++++++++++++ .../nextcloudnative/app/JvmSupportIntake.kt | 6 +-- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt index b9ad9bb57..19c01fd3d 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt @@ -695,6 +695,59 @@ class JvmSupportIntakeTest { Unit } + @Test + fun concurrentCancellationPersistenceNeverRepopulatesPrivatePayload() = runBlocking { + val rejectNextCancellationWrite = AtomicBoolean(false) + val cancellationWriteEntered = CountDownLatch(1) + val allowCancellationWriteFailure = CountDownLatch(1) + val uploadResponseCompleted = CountDownLatch(1) + testFixture( + directorySync = { + if (rejectNextCancellationWrite.compareAndSet(true, false)) { + cancellationWriteEntered.countDown() + assertTrue(allowCancellationWriteFailure.await(3, TimeUnit.SECONDS)) + throw IOException("Synthetic cancellation persistence failure.") + } + }, + afterUploadResponse = { uploadResponseCompleted.countDown() }, + ).use { fixture -> + fixture.server.enqueue( + receiptResponse(fixture.statusUrl).newBuilder().headersDelay(1, TimeUnit.SECONDS).build(), + ) + fixture.server.enqueue(MockResponse.Builder().code(503).build()) + val submission = launch(Dispatchers.Default) { + fixture.intake.submit( + "Private concurrent cancellation note.", + "nightly", + listOf(SupportDiagnosticFieldDraft("private_concurrent_field", "Private concurrent value")), + ) + } + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + rejectNextCancellationWrite.set(true) + val cancellation = launch(Dispatchers.Default) { + assertFalse(fixture.intake.cancel()) + } + assertTrue(cancellationWriteEntered.await(2, TimeUnit.SECONDS)) + assertTrue(uploadResponseCompleted.await(3, TimeUnit.SECONDS)) + + allowCancellationWriteFailure.countDown() + cancellation.join() + submission.join() + + assertIs(fixture.intake.states().value) + val tombstone = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", tombstone.method) + assertEquals(upload.headers["Idempotency-Key"], tombstone.headers["Idempotency-Key"]) + assertFalse(fixture.temporaryRoot.listFiles().orEmpty().any { it.extension == "zip" }) + val descriptor = File(fixture.temporaryRoot, "pending.json").readText() + assertTrue(descriptor.contains(upload.headers["Idempotency-Key"].orEmpty())) + assertTrue(descriptor.contains("\"cancellationPending\":true")) + assertFalse(descriptor.contains("Private concurrent cancellation note.")) + assertFalse(descriptor.contains("private_concurrent_field")) + assertFalse(descriptor.contains("Private concurrent value")) + } + } + @Test fun publishesCancellingWhileAnInterruptedUploadIsReconciled() = runBlocking { testFixture().use { fixture -> diff --git a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt index a93c68d41..afb77e0e0 100644 --- a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt +++ b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt @@ -965,7 +965,7 @@ class JvmSupportIntake( private fun persistMinimalCancellationSafely( submission: PendingSubmission, deleteArchiveAfterPersist: Boolean = true, - ): Boolean { + ): Boolean = synchronized(persistenceLock) { val archive = submission.archive val metadata = submission.metadata val context = submission.context @@ -978,10 +978,10 @@ class JvmSupportIntake( submission.context = context submission.receipt = receipt submission.retryNotBeforeEpochMillis = retryNotBeforeEpochMillis - return false + return@synchronized false } if (deleteArchiveAfterPersist) deletePrivateFileOrRetry(archive) - return true + true } private fun stripPrivateCancellationPayload(submission: PendingSubmission): File? { From 58b332000148ac026f6c7993363fbf77c8733109 Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sat, 15 Aug 2026 05:19:39 +0200 Subject: [PATCH 14/14] fix(support): serialize cancellation before retry upload --- .../app/JvmSupportIntakeTest.kt | 522 +++++++++++++++++- .../nextcloudnative/app/JvmSupportIntake.kt | 336 +++++++++-- 2 files changed, 802 insertions(+), 56 deletions(-) diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt index 19c01fd3d..a6c165a77 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt @@ -559,6 +559,33 @@ class JvmSupportIntakeTest { } } + @Test + fun cancellationAtTheUploadMarkerCannotBeOverwrittenByAStorageRejection() = runBlocking { + val markerEntered = CountDownLatch(1) + val allowMarkerTransition = CountDownLatch(1) + testFixture( + beforeUploadMarker = { + markerEntered.countDown() + check(allowMarkerTransition.await(5, TimeUnit.SECONDS)) + }, + ).use { fixture -> + val submission = launch(Dispatchers.Default) { + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + } + assertTrue(markerEntered.await(5, TimeUnit.SECONDS)) + + assertTrue(fixture.intake.cancel()) + assertIs(fixture.intake.states().value) + + allowMarkerTransition.countDown() + submission.join() + + assertIs(fixture.intake.states().value) + assertEquals(0, fixture.server.requestCount) + assertFalse(File(fixture.temporaryRoot, "pending.json").exists()) + } + } + @Test fun transportGateClosingBeforePostStillAllowsLocalCancellation() = runBlocking { var gateChecks = 0 @@ -582,6 +609,63 @@ class JvmSupportIntakeTest { } } + @Test + fun closedTransportGateCannotReinsertALocallyCancelledSubmission() = runBlocking { + val gateFailureDispositionEntered = CountDownLatch(1) + val allowGateFailureDisposition = CountDownLatch(1) + var gateChecks = 0 + testFixture( + supportMutationsAllowed = { + gateChecks += 1 + gateChecks <= 2 + }, + beforeTransportGateFailureDisposition = { + gateFailureDispositionEntered.countDown() + check(allowGateFailureDisposition.await(5, TimeUnit.SECONDS)) + }, + ).use { fixture -> + val submission = launch(Dispatchers.Default) { + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + } + assertTrue(gateFailureDispositionEntered.await(5, TimeUnit.SECONDS)) + + assertTrue(fixture.intake.cancel()) + assertIs(fixture.intake.states().value) + allowGateFailureDisposition.countDown() + submission.join() + + assertIs(fixture.intake.states().value) + assertEquals(0, fixture.server.requestCount) + assertFalse(File(fixture.temporaryRoot, "pending.json").exists()) + } + } + + @Test + fun cancellationDuringArchiveValidationCompletesCleanly() = runBlocking { + val archiveValidationEntered = CountDownLatch(1) + val allowArchiveValidation = CountDownLatch(1) + testFixture( + beforeUploadArchiveValidation = { + archiveValidationEntered.countDown() + check(allowArchiveValidation.await(5, TimeUnit.SECONDS)) + }, + ).use { fixture -> + val submission = launch(Dispatchers.Default) { + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + } + assertTrue(archiveValidationEntered.await(5, TimeUnit.SECONDS)) + + assertTrue(fixture.intake.cancel()) + assertIs(fixture.intake.states().value) + allowArchiveValidation.countDown() + submission.join() + + assertIs(fixture.intake.states().value) + assertEquals(0, fixture.server.requestCount) + assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) + } + } + @Test fun cancellationWinsBeforeTheUploadCallIsRegistered() = runBlocking { val registrationEntered = CountDownLatch(1) @@ -608,6 +692,218 @@ class JvmSupportIntakeTest { } } + @Test + fun cancellationPendingStopsRetryBeforeAnotherUploadStarts() = runBlocking { + val retryTransitionEntered = CountDownLatch(1) + val allowRetryTransition = CountDownLatch(1) + testFixture( + beforeRetryUploadTransition = { + retryTransitionEntered.countDown() + check(allowRetryTransition.await(5, TimeUnit.SECONDS)) + }, + ).use { fixture -> + fixture.server.enqueue(MockResponse.Builder().onResponseStart(SocketEffect.CloseSocket()).build()) + fixture.server.enqueue(MockResponse.Builder().code(404).build()) + + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + + val retryable = assertIs( + fixture.intake.states().value, + ) + assertFalse(retryable.outcomeAmbiguous) + assertEquals("POST", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) + assertEquals("GET", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) + + val retry = launch(Dispatchers.Default) { fixture.intake.retry() } + assertTrue(retryTransitionEntered.await(5, TimeUnit.SECONDS)) + + assertTrue(fixture.intake.cancel()) + assertIs(fixture.intake.states().value) + allowRetryTransition.countDown() + retry.join() + + assertIs(fixture.intake.states().value) + val cancellation = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", cancellation.method) + assertEquals("/api/v1/receipts", cancellation.url.encodedPath) + assertEquals(3, fixture.server.requestCount) + assertNull(fixture.server.takeRequest(200, TimeUnit.MILLISECONDS)) + } + } + + @Test + fun tombstoneRecoveryAgesFromTheLatestUploadAttempt() = runBlocking { + testFixture().use { fixture -> + fixture.server.enqueue(MockResponse.Builder().onResponseStart(SocketEffect.CloseSocket()).build()) + fixture.server.enqueue(MockResponse.Builder().code(404).build()) + + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + + val retryable = assertIs( + fixture.intake.states().value, + ) + assertFalse(retryable.outcomeAmbiguous) + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("GET", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) + val descriptor = File(fixture.temporaryRoot, "pending.json") + val oldCreatedAt = System.currentTimeMillis() - TimeUnit.DAYS.toMillis(31) + descriptor.writeText( + descriptor.readText().replace( + Regex("\\\"createdAtEpochMillis\\\":\\d+"), + "\"createdAtEpochMillis\":$oldCreatedAt", + ), + ) + fixture.intake.close() + fixture.server.enqueue(receiptResponse(fixture.statusUrl)) + + fixture.newIntake().use { restored -> + assertIs(restored.states().value) + + restored.retry() + + assertIs(restored.states().value) + val retriedUpload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("POST", retriedUpload.method) + assertEquals(upload.headers["Idempotency-Key"], retriedUpload.headers["Idempotency-Key"]) + } + } + } + + @Test + fun cancellationWinsAgainstConcurrentRecoveryExpiry() = runBlocking { + val expiryDispositionEntered = CountDownLatch(1) + val allowExpiryDisposition = CountDownLatch(1) + var nowEpochMillis = System.currentTimeMillis() + testFixture( + currentTimeMillis = { nowEpochMillis }, + beforeRecoveryExpiryDisposition = { + expiryDispositionEntered.countDown() + check(allowExpiryDisposition.await(5, TimeUnit.SECONDS)) + }, + ).use { fixture -> + fixture.server.enqueue(MockResponse.Builder().onResponseStart(SocketEffect.CloseSocket()).build()) + fixture.server.enqueue(MockResponse.Builder().code(404).build()) + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("GET", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) + nowEpochMillis += TimeUnit.DAYS.toMillis(31) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) + + val retry = launch(Dispatchers.Default) { fixture.intake.retry() } + assertTrue(expiryDispositionEntered.await(5, TimeUnit.SECONDS)) + + assertTrue(fixture.intake.cancel()) + assertIs(fixture.intake.states().value) + allowExpiryDisposition.countDown() + retry.join() + + assertIs(fixture.intake.states().value) + val tombstone = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", tombstone.method) + assertEquals(upload.headers["Idempotency-Key"], tombstone.headers["Idempotency-Key"]) + assertFalse(File(fixture.temporaryRoot, "pending.json").exists()) + } + } + + @Test + fun localCancellationDuringRetryPersistenceRemainsTerminal() = runBlocking { + val retryTransitionCompleted = CountDownLatch(1) + val allowRetryPersistence = CountDownLatch(1) + var allowAllMutations = false + var initialGateChecks = 0 + testFixture( + supportMutationsAllowed = { + allowAllMutations || ++initialGateChecks == 1 + }, + afterRetryUploadTransition = { + retryTransitionCompleted.countDown() + check(allowRetryPersistence.await(5, TimeUnit.SECONDS)) + }, + ).use { fixture -> + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + assertIs(fixture.intake.states().value) + assertEquals(0, fixture.server.requestCount) + allowAllMutations = true + + val retry = launch(Dispatchers.Default) { fixture.intake.retry() } + assertTrue(retryTransitionCompleted.await(5, TimeUnit.SECONDS)) + + assertTrue(fixture.intake.cancel()) + assertIs(fixture.intake.states().value) + allowRetryPersistence.countDown() + retry.join() + + assertIs(fixture.intake.states().value) + assertEquals(0, fixture.server.requestCount) + assertFalse(File(fixture.temporaryRoot, "pending.json").exists()) + } + } + + @Test + fun markerWriteFailurePreservesThePriorTombstoneCapability() = runBlocking { + val rejectNextDirectorySync = AtomicBoolean(false) + var markerCount = 0 + testFixture( + directorySync = { + if (rejectNextDirectorySync.compareAndSet(true, false)) { + throw IOException("Synthetic marker persistence failure.") + } + }, + beforeUploadMarker = { + markerCount += 1 + if (markerCount == 2) rejectNextDirectorySync.set(true) + }, + ).use { fixture -> + fixture.server.enqueue(MockResponse.Builder().onResponseStart(SocketEffect.CloseSocket()).build()) + fixture.server.enqueue(MockResponse.Builder().code(404).build()) + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("GET", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) + + fixture.intake.retry() + + assertIs(fixture.intake.states().value) + assertTrue(File(fixture.temporaryRoot, "pending.json").isFile) + assertNull(fixture.server.takeRequest(200, TimeUnit.MILLISECONDS)) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) + + assertTrue(fixture.intake.cancel()) + + assertIs(fixture.intake.states().value) + val tombstone = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", tombstone.method) + assertEquals(upload.headers["Idempotency-Key"], tombstone.headers["Idempotency-Key"]) + assertFalse(File(fixture.temporaryRoot, "pending.json").exists()) + } + } + + @Test + fun cancellationBeforePendingInstallRemainsLocal() = runBlocking { + val pendingInstallEntered = CountDownLatch(1) + val allowPendingInstall = CountDownLatch(1) + testFixture( + beforePendingSubmissionInstall = { + pendingInstallEntered.countDown() + check(allowPendingInstall.await(5, TimeUnit.SECONDS)) + }, + ).use { fixture -> + val submission = launch(Dispatchers.Default) { + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + } + assertTrue(pendingInstallEntered.await(5, TimeUnit.SECONDS)) + + assertTrue(fixture.intake.cancel()) + assertIs(fixture.intake.states().value) + allowPendingInstall.countDown() + submission.join() + + assertIs(fixture.intake.states().value) + assertEquals(0, fixture.server.requestCount) + assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) + } + } + @Test fun publishesBusyStateAndCancelsBeforeSubmissionPreparationCompletes() = runBlocking { val preparationEntered = CountDownLatch(1) @@ -635,6 +931,32 @@ class JvmSupportIntakeTest { } } + @Test + fun cancellationDuringArchivePromotionRemainsTerminal() = runBlocking { + val promotionEntered = CountDownLatch(1) + val allowPromotion = CountDownLatch(1) + testFixture( + beforeArchivePromotion = { + promotionEntered.countDown() + check(allowPromotion.await(5, TimeUnit.SECONDS)) + }, + ).use { fixture -> + val submission = launch(Dispatchers.Default) { + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + } + assertTrue(promotionEntered.await(5, TimeUnit.SECONDS)) + + assertTrue(fixture.intake.cancel()) + assertIs(fixture.intake.states().value) + allowPromotion.countDown() + submission.join() + + assertIs(fixture.intake.states().value) + assertEquals(0, fixture.server.requestCount) + assertTrue(fixture.temporaryRoot.listFiles().orEmpty().isEmpty()) + } + } + @Test fun preservesPreparationBlockAcrossAccountSwitchesUntilTheOperationEnds() = runBlocking { val preparationEntered = CountDownLatch(1) @@ -1097,6 +1419,38 @@ class JvmSupportIntakeTest { } } + @Test + fun cancellationBeforeReceiptRegistrationStartsTheTombstoneImmediately() = runBlocking { + val receiptRegistrationEntered = CountDownLatch(1) + val allowReceiptRegistration = CountDownLatch(1) + testFixture( + beforeReceiptCallRegistration = { + receiptRegistrationEntered.countDown() + check(allowReceiptRegistration.await(5, TimeUnit.SECONDS)) + }, + ).use { fixture -> + fixture.server.enqueue(MockResponse.Builder().onResponseStart(SocketEffect.CloseSocket()).build()) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) + val submission = launch(Dispatchers.Default) { + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + } + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertTrue(receiptRegistrationEntered.await(5, TimeUnit.SECONDS)) + + assertTrue(fixture.intake.cancel()) + assertIs(fixture.intake.states().value) + allowReceiptRegistration.countDown() + submission.join() + + assertIs(fixture.intake.states().value) + val tombstone = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", tombstone.method) + assertEquals(upload.headers["Idempotency-Key"], tombstone.headers["Idempotency-Key"]) + assertEquals(2, fixture.server.requestCount) + assertNull(fixture.server.takeRequest(200, TimeUnit.MILLISECONDS)) + } + } + @Test fun cancellationAfterReceiptLookupCompletesSendsAuthoritativeTombstone() = runBlocking { val lookupCompleted = CountDownLatch(1) @@ -1133,6 +1487,51 @@ class JvmSupportIntakeTest { } } + @Test + fun cancellationDoesNotCancelTheTombstoneStartedByResponseHandling() = runBlocking { + val uploadResponseCompleted = CountDownLatch(1) + val allowUploadResponseDisposition = CountDownLatch(1) + val cancellationIntentPublished = CountDownLatch(1) + val allowCancellationContinuation = CountDownLatch(1) + testFixture( + afterUploadResponse = { + uploadResponseCompleted.countDown() + assertTrue(allowUploadResponseDisposition.await(5, TimeUnit.SECONDS)) + }, + afterCancellationIntentPublished = { + cancellationIntentPublished.countDown() + assertTrue(allowCancellationContinuation.await(5, TimeUnit.SECONDS)) + }, + ).use { fixture -> + fixture.server.enqueue(MockResponse.Builder().code(503).build()) + fixture.server.enqueue( + MockResponse.Builder().code(204).headersDelay(2, TimeUnit.SECONDS).build(), + ) + val submission = launch(Dispatchers.Default) { + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + } + assertEquals("POST", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) + assertTrue(uploadResponseCompleted.await(5, TimeUnit.SECONDS)) + + val cancellation = launch(Dispatchers.Default) { + assertTrue(fixture.intake.cancel()) + } + assertTrue(cancellationIntentPublished.await(5, TimeUnit.SECONDS)) + allowUploadResponseDisposition.countDown() + val tombstone = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", tombstone.method) + assertEquals("/api/v1/receipts", tombstone.url.encodedPath) + allowCancellationContinuation.countDown() + + cancellation.join() + submission.join() + + assertIs(fixture.intake.states().value) + assertEquals(2, fixture.server.requestCount) + assertFalse(File(fixture.temporaryRoot, "pending.json").exists()) + } + } + @Test fun cancellationAfterReceiptIntentCheckContinuesWithAuthoritativeTombstone() = runBlocking { val dispositionEntered = CountDownLatch(1) @@ -1167,6 +1566,41 @@ class JvmSupportIntakeTest { } } + @Test + fun cancellationWinsAgainstPermanentUploadRejectionDisposition() = runBlocking { + val responseDispositionEntered = CountDownLatch(1) + val allowResponseDisposition = CountDownLatch(1) + testFixture( + beforeUploadResponseDisposition = { + responseDispositionEntered.countDown() + check(allowResponseDisposition.await(5, TimeUnit.SECONDS)) + }, + ).use { fixture -> + fixture.server.enqueue( + MockResponse.Builder().code(400).body( + """{"contractVersion":1,"code":"invalid_report","message":"Invalid."}""", + ).build(), + ) + fixture.server.enqueue(MockResponse.Builder().code(204).build()) + val submission = launch(Dispatchers.Default) { + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + } + val upload = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertTrue(responseDispositionEntered.await(5, TimeUnit.SECONDS)) + + assertTrue(fixture.intake.cancel()) + assertIs(fixture.intake.states().value) + allowResponseDisposition.countDown() + submission.join() + + assertIs(fixture.intake.states().value) + val tombstone = requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)) + assertEquals("DELETE", tombstone.method) + assertEquals(upload.headers["Idempotency-Key"], tombstone.headers["Idempotency-Key"]) + assertFalse(File(fixture.temporaryRoot, "pending.json").exists()) + } + } + @Test fun cancellationAfterUploadResponseCompletesSendsAuthoritativeTombstone() = runBlocking { val responseCompleted = CountDownLatch(1) @@ -1369,7 +1803,7 @@ class JvmSupportIntakeTest { ) val restored = fixture.newIntake() assertIs(restored.states().value) - fixture.server.enqueue(MockResponse.Builder().code(410).build()) + fixture.server.enqueue(submissionCancelledResponse()) restored.retry() @@ -1383,6 +1817,44 @@ class JvmSupportIntakeTest { } } + @Test + fun unverifiedGoneUploadResponseRetainsRecovery() = runBlocking { + testFixture().use { fixture -> + fixture.server.enqueue( + MockResponse.Builder().code(410).body( + """{"contractVersion":1,"code":"not_found","message":"Gone."}""", + ).build(), + ) + + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + + val retryable = assertIs( + fixture.intake.states().value, + ) + assertTrue(retryable.outcomeAmbiguous) + assertTrue(File(fixture.temporaryRoot, "pending.json").isFile) + assertEquals("POST", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) + } + } + + @Test + fun unverifiedGoneReceiptResponseRetainsRecovery() = runBlocking { + testFixture().use { fixture -> + fixture.server.enqueue(MockResponse.Builder().onResponseStart(SocketEffect.CloseSocket()).build()) + fixture.server.enqueue(MockResponse.Builder().code(410).body("gone").build()) + + fixture.intake.submit("A refresh failed.", "nightly", emptyList()) + + val retryable = assertIs( + fixture.intake.states().value, + ) + assertTrue(retryable.outcomeAmbiguous) + assertTrue(File(fixture.temporaryRoot, "pending.json").isFile) + assertEquals("POST", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) + assertEquals("GET", requireNotNull(fixture.server.takeRequest(2, TimeUnit.SECONDS)).method) + } + } + @Test fun cancellationRetainsRecoveryUntilTerminalNoContent() = runBlocking { testFixture().use { fixture -> @@ -2077,8 +2549,18 @@ class JvmSupportIntakeTest { descriptorCleanupRetryMillis: Long = 60_000L, beforeCallRegistration: () -> Unit = {}, beforeSubmissionPreparation: () -> Unit = {}, + beforePendingSubmissionInstall: () -> Unit = {}, beforeBundlePackaging: () -> Unit = {}, afterBundlePackaging: () -> Unit = {}, + beforeArchivePromotion: () -> Unit = {}, + beforeRetryUploadTransition: () -> Unit = {}, + afterRetryUploadTransition: () -> Unit = {}, + beforeRecoveryExpiryDisposition: () -> Unit = {}, + beforeUploadMarker: () -> Unit = {}, + beforeUploadArchiveValidation: () -> Unit = {}, + beforeTransportGateFailureDisposition: () -> Unit = {}, + beforeReceiptCallRegistration: () -> Unit = {}, + afterCancellationIntentPublished: () -> Unit = {}, afterUploadResponse: () -> Unit = {}, beforeUploadResponseDisposition: () -> Unit = {}, afterReceiptLookup: () -> Unit = {}, @@ -2091,6 +2573,7 @@ class JvmSupportIntakeTest { archiveTemporaryBeforeInitialization: Boolean = false, invalidPendingBeforeInitialization: Boolean = false, invalidCompletedBeforeInitialization: Boolean = false, + currentTimeMillis: () -> Long = System::currentTimeMillis, ): Fixture { val root = createTempDirectory("support-intake-test").toFile() val diagnosticRoot = File(root, "diagnostics") @@ -2145,8 +2628,18 @@ class JvmSupportIntakeTest { descriptorCleanupRetryMillis = descriptorCleanupRetryMillis, beforeCallRegistration = beforeCallRegistration, beforeSubmissionPreparation = beforeSubmissionPreparation, + beforePendingSubmissionInstall = beforePendingSubmissionInstall, beforeBundlePackaging = beforeBundlePackaging, afterBundlePackaging = afterBundlePackaging, + beforeArchivePromotion = beforeArchivePromotion, + beforeRetryUploadTransition = beforeRetryUploadTransition, + afterRetryUploadTransition = afterRetryUploadTransition, + beforeRecoveryExpiryDisposition = beforeRecoveryExpiryDisposition, + beforeUploadMarker = beforeUploadMarker, + beforeUploadArchiveValidation = beforeUploadArchiveValidation, + beforeTransportGateFailureDisposition = beforeTransportGateFailureDisposition, + beforeReceiptCallRegistration = beforeReceiptCallRegistration, + afterCancellationIntentPublished = afterCancellationIntentPublished, afterUploadResponse = afterUploadResponse, beforeUploadResponseDisposition = beforeUploadResponseDisposition, afterReceiptLookup = afterReceiptLookup, @@ -2154,6 +2647,7 @@ class JvmSupportIntakeTest { privateFileDelete = privateFileDelete, pendingDescriptorRead = pendingDescriptorRead, completedDescriptorRead = completedDescriptorRead, + currentTimeMillis = currentTimeMillis, ) } @@ -2182,6 +2676,10 @@ class JvmSupportIntakeTest { ).build() } + private fun submissionCancelledResponse(): MockResponse = MockResponse.Builder().code(410).body( + """{"contractVersion":1,"code":"submission_cancelled","message":"Submission cancelled."}""", + ).build() + private data class Fixture( val root: File, val temporaryRoot: File, @@ -2193,8 +2691,18 @@ class JvmSupportIntakeTest { val descriptorCleanupRetryMillis: Long, val beforeCallRegistration: () -> Unit, val beforeSubmissionPreparation: () -> Unit, + val beforePendingSubmissionInstall: () -> Unit, val beforeBundlePackaging: () -> Unit, val afterBundlePackaging: () -> Unit, + val beforeArchivePromotion: () -> Unit, + val beforeRetryUploadTransition: () -> Unit, + val afterRetryUploadTransition: () -> Unit, + val beforeRecoveryExpiryDisposition: () -> Unit, + val beforeUploadMarker: () -> Unit, + val beforeUploadArchiveValidation: () -> Unit, + val beforeTransportGateFailureDisposition: () -> Unit, + val beforeReceiptCallRegistration: () -> Unit, + val afterCancellationIntentPublished: () -> Unit, val afterUploadResponse: () -> Unit, val beforeUploadResponseDisposition: () -> Unit, val afterReceiptLookup: () -> Unit, @@ -2202,6 +2710,7 @@ class JvmSupportIntakeTest { val privateFileDelete: (File) -> Boolean, val pendingDescriptorRead: (File) -> String, val completedDescriptorRead: (File) -> String, + val currentTimeMillis: () -> Long, ) : AutoCloseable { val intake = newIntake() val statusUrl: String get() = server.url("/r/abcdefghijklmnopqrstuvwxyzABCDEFGH_12345678").toString() @@ -2220,8 +2729,18 @@ class JvmSupportIntakeTest { descriptorCleanupRetryMillis = descriptorCleanupRetryMillis, beforeCallRegistration = beforeCallRegistration, beforeSubmissionPreparation = beforeSubmissionPreparation, + beforePendingSubmissionInstall = beforePendingSubmissionInstall, beforeBundlePackaging = beforeBundlePackaging, afterBundlePackaging = afterBundlePackaging, + beforeArchivePromotion = beforeArchivePromotion, + beforeRetryUploadTransition = beforeRetryUploadTransition, + afterRetryUploadTransition = afterRetryUploadTransition, + beforeRecoveryExpiryDisposition = beforeRecoveryExpiryDisposition, + beforeUploadMarker = beforeUploadMarker, + beforeUploadArchiveValidation = beforeUploadArchiveValidation, + beforeTransportGateFailureDisposition = beforeTransportGateFailureDisposition, + beforeReceiptCallRegistration = beforeReceiptCallRegistration, + afterCancellationIntentPublished = afterCancellationIntentPublished, afterUploadResponse = afterUploadResponse, beforeUploadResponseDisposition = beforeUploadResponseDisposition, afterReceiptLookup = afterReceiptLookup, @@ -2229,6 +2748,7 @@ class JvmSupportIntakeTest { privateFileDelete = privateFileDelete, pendingDescriptorRead = pendingDescriptorRead, completedDescriptorRead = completedDescriptorRead, + currentTimeMillis = currentTimeMillis, ).also { intake -> intake.setActiveAccountIdentity(TEST_ACCOUNT_IDENTITY) runBlocking { intake.awaitInitialization() } diff --git a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt index afb77e0e0..5209bab53 100644 --- a/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt +++ b/ui/src/jvmMain/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntake.kt @@ -64,8 +64,18 @@ class JvmSupportIntake( private val descriptorCleanupRetryMillis: Long = SUPPORT_DESCRIPTOR_DELETE_RETRY_MILLIS, private val beforeCallRegistration: () -> Unit = {}, private val beforeSubmissionPreparation: () -> Unit = {}, + private val beforePendingSubmissionInstall: () -> Unit = {}, private val beforeBundlePackaging: () -> Unit = {}, private val afterBundlePackaging: () -> Unit = {}, + private val beforeArchivePromotion: () -> Unit = {}, + private val beforeRetryUploadTransition: () -> Unit = {}, + private val afterRetryUploadTransition: () -> Unit = {}, + private val beforeRecoveryExpiryDisposition: () -> Unit = {}, + private val beforeUploadMarker: () -> Unit = {}, + private val beforeUploadArchiveValidation: () -> Unit = {}, + private val beforeTransportGateFailureDisposition: () -> Unit = {}, + private val beforeReceiptCallRegistration: () -> Unit = {}, + private val afterCancellationIntentPublished: () -> Unit = {}, private val afterUploadResponse: () -> Unit = {}, private val beforeUploadResponseDisposition: () -> Unit = {}, private val afterReceiptLookup: () -> Unit = {}, @@ -77,6 +87,7 @@ class JvmSupportIntake( private val completedDescriptorRead: (File) -> String = { descriptor -> descriptor.readText(Charsets.UTF_8) }, + private val currentTimeMillis: () -> Long = System::currentTimeMillis, ) : AutoCloseable { private val baseUrl = supportBaseUrl.toHttpUrl() private val client = client.newBuilder() @@ -221,14 +232,28 @@ class JvmSupportIntake( }, ), idempotencyKey = secureIdempotencyKey(), - createdAtEpochMillis = System.currentTimeMillis().coerceAtLeast(0L), + createdAtEpochMillis = currentTimeMillis().coerceAtLeast(0L), originAccountIdentity = originAccountIdentity, - cancellationPending = cancellationRequested.get(), context = context, ) - synchronized(lock) { pending = submission } + beforePendingSubmissionInstall() + val submissionInstalled = synchronized(lock) { + if (cancellationRequested.get()) { + false + } else { + pending = submission + true + } + } + if (!submissionInstalled) { + publishState(SupportDiagnosticsSubmissionState.Cancelling, originAccountIdentity) + return@withContext + } if (!persistPendingSafely(submission)) { - finishRejected(submission, "The private support submission could not be retained safely on this device.") + finishRejectedIfCurrent( + submission, + "The private support submission could not be retained safely on this device.", + ) return@withContext } if (!packageSubmission(submission)) return@withContext @@ -275,14 +300,18 @@ class JvmSupportIntake( SupportDiagnosticsSubmissionState.Cancelling, submission.originAccountIdentity, ) - cancelPendingSubmission(submission) + finishOrReconcileCancellation(submission) return@withContext } - if (submission.recoveryExpired(System.currentTimeMillis())) { - finishRejected(submission, "The private report recovery capability expired and was removed from this device.") + if (submission.recoveryExpired(currentTimeMillis())) { + beforeRecoveryExpiryDisposition() + finishRejectedIfCurrent( + submission, + "The private report recovery capability expired and was removed from this device.", + ) return@withContext } - val waitMillis = submission.retryNotBeforeEpochMillis?.minus(System.currentTimeMillis()) ?: 0L + val waitMillis = submission.retryNotBeforeEpochMillis?.minus(currentTimeMillis()) ?: 0L if (waitMillis > 0L) { publishState(SupportDiagnosticsSubmissionState.RetryableFailure( "Obiente Support asked the app to wait before retrying. Try again shortly.", @@ -297,15 +326,35 @@ class JvmSupportIntake( ) return@withContext } - cancellationRequested.set(false) + beforeRetryUploadTransition() + val continueRetry = synchronized(lock) { + when { + pending !== submission -> false + submission.cancellationPending || cancellationRequested.get() -> false + else -> { + cancellationRequested.set(false) + true + } + } + } + if (!continueRetry) { + if (synchronized(lock) { pending === submission }) { + finishOrReconcileCancellation(submission) + } + return@withContext + } + afterRetryUploadTransition() submission.retryNotBeforeEpochMillis = null if (!persistPendingSafely(submission)) { - finishRejected(submission, "The private support submission could not be retained safely on this device.") + finishRejectedIfCurrent( + submission, + "The private support submission could not be retained safely on this device.", + ) return@withContext } if (submission.archive == null && !packageSubmission(submission)) return@withContext if (submission.archive?.isFile != true) { - finishRejected(submission, "The pending private report archive is unavailable.") + finishRejectedIfCurrent(submission, "The pending private report archive is unavailable.") return@withContext } upload(submission) @@ -357,12 +406,14 @@ class JvmSupportIntake( suspend fun cancel(): Boolean { awaitInitialization() + var callAtIntent: Call? = null // Serialize the terminal receipt decision with publication of the user's intent. If receipt // completion wins and clears pending first, cancellation is correctly reported as too late. val pendingCancellation: Boolean? = synchronized(lock) { val submission = pending when { submission?.belongsTo(activeAccountIdentity) == true -> { + callAtIntent = activeCall.get() cancellationRequested.set(true) true } @@ -377,7 +428,10 @@ class JvmSupportIntake( return when (pendingCancellation) { null -> false false -> true - true -> withContext(Dispatchers.IO) { cancelAfterIntentPublished() } + true -> { + afterCancellationIntentPublished() + withContext(Dispatchers.IO) { cancelAfterIntentPublished(callAtIntent) } + } } } @@ -414,17 +468,27 @@ class JvmSupportIntake( } } - private fun cancelAfterIntentPublished(): Boolean { - val submission = synchronized(lock) { pending } + private fun cancelAfterIntentPublished(callAtIntent: Call?): Boolean { + var localCancellationCommitted = false + val submission = synchronized(lock) { + pending?.also { current -> + if (!current.cancellationRequiresTombstone && activeCall.get() == null) { + // Claim the terminal decision while holding the same lock used by the upload + // marker transition. The upload cannot publish a tombstone requirement after + // cancellation has already removed this submission from the active state. + pending = null + localCancellationCommitted = true + } + } + } if (submission != null) { - if (!submission.cancellationRequiresTombstone && activeCall.get() == null) { + if (localCancellationCommitted) { finishCancelled(submission) return true } submission.cancellationPending = true submission.outcomeAmbiguous = true val archive = submission.archive - val callAtIntent = activeCall.get() val cancellationPersisted = persistMinimalCancellationSafely( submission, deleteArchiveAfterPersist = false, @@ -629,7 +693,7 @@ class JvmSupportIntake( private suspend fun packageSubmission(submission: PendingSubmission): Boolean { if (cancellationRequested.get()) { - finishCancelled(submission) + finishOrReconcileCancellation(submission) return false } publishState(SupportDiagnosticsSubmissionState.Packaging) @@ -642,7 +706,7 @@ class JvmSupportIntake( } catch (cancellation: CancellationException) { deletePrivateFileOrRetry(destination) if (cancellationRequested.get() || synchronized(lock) { pending !== submission }) { - finishCancelled(submission) + finishOrReconcileCancellation(submission) } else { retainForRetry( submission, @@ -654,7 +718,7 @@ class JvmSupportIntake( } catch (_: Throwable) { deletePrivateFileOrRetry(destination) if (cancellationRequested.get() || synchronized(lock) { pending !== submission }) { - finishCancelled(submission) + finishOrReconcileCancellation(submission) } else { retainForRetry( submission, @@ -679,13 +743,28 @@ class JvmSupportIntake( deletePrivateFileOrRetry(prepared.archive) return false } - submission.archive = prepared.archive + beforeArchivePromotion() + val archivePromoted = synchronized(lock) { + if (pending !== submission || cancellationRequested.get() || submission.cancellationPending) { + false + } else { + submission.archive = prepared.archive + true + } + } + if (!archivePromoted) { + deletePrivateFileOrRetry(prepared.archive) + return false + } if (!persistPendingSafely(submission)) { - finishRejected(submission, "The private support submission could not be retained safely on this device.") + finishRejectedIfCurrent( + submission, + "The private support submission could not be retained safely on this device.", + ) return false } if (cancellationRequested.get()) { - finishCancelled(submission) + finishOrReconcileCancellation(submission) return false } return true @@ -693,12 +772,12 @@ class JvmSupportIntake( private suspend fun upload(submission: PendingSubmission) { if (cancellationRequested.get()) { - finishCancelled(submission) + finishOrReconcileCancellation(submission) return } val mutationAllowedBeforePreparation = supportMutationsAreAllowed() if (cancellationRequested.get() || synchronized(lock) { pending !== submission }) { - finishCancelled(submission) + finishOrReconcileCancellation(submission) return } if (!mutationAllowedBeforePreparation) { @@ -706,7 +785,13 @@ class JvmSupportIntake( return } val archive = requireNotNull(submission.archive) { "The private support archive has not been prepared." } - require(archive.isFile && archive.length() in 1L..MAX_SUPPORT_ARCHIVE_BYTES) + beforeUploadArchiveValidation() + if (!archive.isFile || archive.length() !in 1L..MAX_SUPPORT_ARCHIVE_BYTES) { + if (synchronized(lock) { pending === submission }) { + finishRejectedIfCurrent(submission, "The pending private report archive is unavailable.") + } + return + } val metadata = json.encodeToString(SupportIntakeMetadata.serializer(), submission.metadata) val progressBody = ProgressRequestBody( delegate = archive.asRequestBody(SUPPORT_ARCHIVE_MEDIA_TYPE), @@ -731,18 +816,61 @@ class JvmSupportIntake( .build() val mutationAllowedAtTransport = supportMutationsAreAllowed() if (cancellationRequested.get() || synchronized(lock) { pending !== submission }) { - finishCancelled(submission) + finishOrReconcileCancellation(submission) return } if (!mutationAllowedAtTransport) { + beforeTransportGateFailureDisposition() retainForRetry(submission, READ_ONLY_SUPPORT_MESSAGE, ambiguous = false) return } - submission.latestUploadAttemptAtEpochMillis = System.currentTimeMillis().coerceAtLeast(0L) - submission.outcomeAmbiguous = true - submission.cancellationRequiresTombstone = true + beforeUploadMarker() + var previousLatestUploadAttemptAtEpochMillis: Long? = null + var previousOutcomeAmbiguous = false + var previousCancellationRequiresTombstone = false + val uploadMarked = synchronized(lock) { + if (pending !== submission || cancellationRequested.get() || submission.cancellationPending) { + false + } else { + previousLatestUploadAttemptAtEpochMillis = submission.latestUploadAttemptAtEpochMillis + previousOutcomeAmbiguous = submission.outcomeAmbiguous + previousCancellationRequiresTombstone = submission.cancellationRequiresTombstone + submission.latestUploadAttemptAtEpochMillis = currentTimeMillis().coerceAtLeast(0L) + submission.outcomeAmbiguous = true + submission.cancellationRequiresTombstone = true + true + } + } + if (!uploadMarked) { + if (synchronized(lock) { pending === submission }) { + finishOrReconcileCancellation(submission) + } + return + } if (!persistPendingSafely(submission)) { - finishRejected(submission, "The private support submission could not be retained safely on this device.") + val stillPending = synchronized(lock) { + if (pending !== submission) { + false + } else { + submission.latestUploadAttemptAtEpochMillis = previousLatestUploadAttemptAtEpochMillis + submission.outcomeAmbiguous = previousOutcomeAmbiguous + submission.cancellationRequiresTombstone = previousCancellationRequiresTombstone + true + } + } + if (stillPending) { + if (previousCancellationRequiresTombstone) { + publishState(SupportDiagnosticsSubmissionState.RetryableFailure( + "The retry could not be stored safely. The earlier upload remains recoverable on this device.", + outcomeAmbiguous = previousOutcomeAmbiguous, + )) + } else { + finishRejectedIfCurrent( + submission, + "The private support submission could not be retained safely on this device.", + ) + } + } return } publishState(SupportDiagnosticsSubmissionState.Uploading(0f)) @@ -750,7 +878,16 @@ class JvmSupportIntake( beforeCallRegistration() if (!registerActiveCall(submission, call, allowCancellationRequested = false)) { call.cancel() + synchronized(lock) { + if (pending === submission) { + submission.latestUploadAttemptAtEpochMillis = previousLatestUploadAttemptAtEpochMillis + submission.outcomeAmbiguous = previousOutcomeAmbiguous + submission.cancellationRequiresTombstone = previousCancellationRequiresTombstone + } + } when { + cancellationRequested.get() && previousCancellationRequiresTombstone -> + finishOrReconcileCancellation(submission) cancellationRequested.get() -> finishCancelled(submission) synchronized(lock) { pending === submission } -> retainForRetry( submission, @@ -782,8 +919,16 @@ class JvmSupportIntake( ambiguous = false, retryNotBeforeEpochMillis = response.retryNotBeforeEpochMillis(), ) - response.code == 410 -> finishCancelled(submission) - response.code in 400..499 -> finishRejected(submission, decodeProblem(responseText)) + response.code == 410 && isSubmissionCancelledProblem(responseText) -> finishCancelled(submission) + response.code == 410 -> retainForRetry( + submission, + "Obiente Support returned an unverified cancellation result. Retry safely to reconcile the report.", + ambiguous = true, + ) + response.code in 400..499 -> finishRejectedIfCurrent( + submission, + decodeProblem(responseText), + ) else -> retainForRetry( submission, "Obiente Support is temporarily unavailable.", @@ -828,15 +973,14 @@ class JvmSupportIntake( .get() .build() val call = client.newCall(request) - if (!registerActiveCall(submission, call, allowCancellationRequested = true)) { + beforeReceiptCallRegistration() + if (!registerActiveCall(submission, call, allowCancellationRequested = false)) { call.cancel() - if (synchronized(lock) { pending === submission }) { - retainForRetry( - submission, - "The upload result still needs to be reconciled. You can retry it safely.", - ambiguous = true, - ) - } + retainForRetry( + submission, + "The upload result still needs to be reconciled. You can retry it safely.", + ambiguous = true, + ) return } val responseResult = try { @@ -878,7 +1022,12 @@ class JvmSupportIntake( } responseCode == 404 -> retainForRetry(submission, "The upload did not complete. You can retry it safely.", false) - responseCode == 410 -> finishCancelled(submission) + responseCode == 410 && isSubmissionCancelledProblem(responseText) -> finishCancelled(submission) + responseCode == 410 -> retainForRetry( + submission, + "Obiente Support returned an unverified cancellation result. Retry safely to reconcile the report.", + ambiguous = true, + ) else -> retainForRetry( submission, "The upload result is uncertain. Check your connection before retrying.", @@ -1145,6 +1294,22 @@ class JvmSupportIntake( publishState(SupportDiagnosticsSubmissionState.Rejected(message), submission.originAccountIdentity) } + private fun finishRejectedIfCurrent(submission: PendingSubmission, message: String) { + val rejectionCommitted = synchronized(lock) { + if ( + pending === submission && + !cancellationRequested.get() && + !submission.cancellationPending + ) { + pending = null + true + } else { + false + } + } + if (rejectionCommitted) finishRejected(submission, message) + } + private fun finishCancelled(submission: PendingSubmission) { finishTerminal(submission) publishState( @@ -1157,24 +1322,70 @@ class JvmSupportIntake( ) } + private fun finishOrReconcileCancellation(submission: PendingSubmission) { + if ( + synchronized(lock) { + pending === submission && submission.cancellationRequiresTombstone + } + ) { + cancelPendingSubmission(submission) + } else { + finishCancelled(submission) + } + } + private fun retainForRetry( submission: PendingSubmission, message: String, ambiguous: Boolean, retryNotBeforeEpochMillis: Long? = null, ) { - submission.outcomeAmbiguous = ambiguous - submission.retryNotBeforeEpochMillis = retryNotBeforeEpochMillis - synchronized(lock) { pending = submission } - if (persistPendingSafely(submission)) { - publishState(SupportDiagnosticsSubmissionState.RetryableFailure(message, ambiguous)) - } else { - // Atomic replacement keeps the descriptor from immediately before the request. That - // record retains the idempotency key and conservatively requires reconciliation. - publishState(SupportDiagnosticsSubmissionState.RetryableFailure( - "The updated retry state could not be stored. Keep the app open and retry safely to reconcile the report.", - outcomeAmbiguous = true, - )) + val retentionCommitted = synchronized(lock) { + if ( + pending !== submission || + cancellationRequested.get() || + submission.cancellationPending + ) { + false + } else { + submission.outcomeAmbiguous = ambiguous + submission.retryNotBeforeEpochMillis = retryNotBeforeEpochMillis + true + } + } + if (!retentionCommitted) { + if (synchronized(lock) { pending === submission }) { + finishOrReconcileCancellation(submission) + } + return + } + val persisted = persistPendingSafely(submission) + val retryStatePublished = synchronized(lock) { + if ( + pending === submission && + !cancellationRequested.get() && + !submission.cancellationPending + ) { + publishStateLocked( + if (persisted) { + SupportDiagnosticsSubmissionState.RetryableFailure(message, ambiguous) + } else { + // Atomic replacement keeps the descriptor from immediately before the + // request, including its idempotency key and recovery requirement. + SupportDiagnosticsSubmissionState.RetryableFailure( + "The updated retry state could not be stored. Keep the app open and retry safely to reconcile the report.", + outcomeAmbiguous = true, + ) + }, + submission.originAccountIdentity, + ) + true + } else { + false + } + } + if (!retryStatePublished && synchronized(lock) { pending === submission }) { + finishOrReconcileCancellation(submission) } } @@ -1215,6 +1426,13 @@ class JvmSupportIntake( ?.takeIf(String::isNotBlank) ?: "Obiente Support rejected this diagnostic report." + private fun isSubmissionCancelledProblem(response: String): Boolean = runCatching { + json.decodeFromString(SupportIntakeProblem.serializer(), response).let { problem -> + problem.contractVersion == SUPPORT_INTAKE_CONTRACT_VERSION && + problem.code == SUPPORT_SUBMISSION_CANCELLED_CODE + } + }.getOrDefault(false) + private fun pruneTemporaryReports(retainedArchive: File?) { val cutoff = System.currentTimeMillis() - SUPPORT_TEMPORARY_MAX_AGE_MILLIS temporaryRoot.listFiles().orEmpty() @@ -1401,7 +1619,7 @@ class JvmSupportIntake( } val recoveryDeadlineEpochMillis = persisted.receipt ?.let { receipt -> Instant.parse(receipt.retentionUntil).toEpochMilli() } - ?: if (persisted.outcomeAmbiguous) { + ?: if (persisted.outcomeAmbiguous || persisted.cancellationRequiresTombstone == true) { (persisted.latestUploadAttemptAtEpochMillis ?: persisted.createdAtEpochMillis) .saturatingAdd(SUPPORT_RECOVERY_MAX_AGE_MILLIS) } else { @@ -1649,7 +1867,7 @@ class JvmSupportIntake( fun recoveryExpired(nowEpochMillis: Long): Boolean { val deadline = receipt ?.let { value -> runCatching { Instant.parse(value.retentionUntil).toEpochMilli() }.getOrNull() } - ?: if (outcomeAmbiguous) { + ?: if (outcomeAmbiguous || cancellationRequiresTombstone) { (latestUploadAttemptAtEpochMillis ?: createdAtEpochMillis) .saturatingAdd(SUPPORT_RECOVERY_MAX_AGE_MILLIS) } else { @@ -1670,6 +1888,13 @@ class JvmSupportIntake( fun isRetained(nowEpochMillis: Long): Boolean = nowEpochMillis <= retentionUntilEpochMillis } + @Serializable + private data class SupportIntakeProblem( + val contractVersion: Int, + val code: String, + val message: String, + ) + @Serializable private data class PersistedPendingSubmission( val archiveName: String?, @@ -1943,6 +2168,7 @@ private val SUPPORT_IDEMPOTENCY_PATTERN = Regex("[A-Za-z0-9_-]{43}") private val SUPPORT_ACCOUNT_IDENTITY_PATTERN = Regex("[0-9a-f]{32}(?:[0-9a-f]{32})?") private val RETRYABLE_CLIENT_STATUS_CODES = setOf(425, 429) private val TERMINAL_DELETION_STATUS_CODES = setOf(200, 204) +private const val SUPPORT_SUBMISSION_CANCELLED_CODE = "submission_cancelled" private const val MAX_SUPPORT_INTAKE_MESSAGE_LENGTH = 240 private const val MAX_SUPPORT_INTAKE_RESPONSE_BYTES = 64 * 1024 private const val MAX_SUPPORT_INTAKE_DESCRIPTION_BYTES = 8_000