diff --git a/ADAPTER_ARCHITECTURE.md b/ADAPTER_ARCHITECTURE.md index 2a9984690..8ec2b3cb9 100644 --- a/ADAPTER_ARCHITECTURE.md +++ b/ADAPTER_ARCHITECTURE.md @@ -362,6 +362,33 @@ dialog owns only the temporary name and destination choice. The parent captures an immutable validated destination for the typed mutation, and the transport keeps the source ETag and no-overwrite preconditions. +### Local follow-up after file writes + +The server result of a file write is authoritative. Folder and text file +creation, text saves, favorites, version restores, and rename, move, copy, and +delete return it with a separate `FileMutationLocalFollowUp`, and surfaces +show an incomplete one with the shared `reportFor` copy instead of plain +success. + +- Desktop writes go through `DesktopFileWrites`. Local cache and virtual-file + bookkeeping can wait on the virtual-file provider lock, which Windows Cloud + Files activation, recovery, or storage summaries can hold for a long time, + so it never runs on the caller's coroutine. `DesktopFileMutationFollowUps` queues it when the server + accepts the request or its result becomes unknown, refreshes one path at a + time, and coalesces repeated paths. The caller waits a bounded time and + receives `StillRunning` after that; caller cancellation leaves the queued + refresh running. +- A full queue reports `Failed`, whose copy asks for a folder refresh, and an + unknown result whose reconciliation could not be queued says so. Folder sync + uploads and Linux mount writes have no waiting caller: they queue the same + refresh without blocking, and refresh on their own thread when the queue is + full rather than dropping it. +- Sign-out and account removal abandon the account's queued refreshes; waiting + callers see `Failed`, and a refresh that was already running cannot change + that result when it finishes. +- Android invalidates its read cache after the confirmed write and reports a + failed invalidation the same way. + ### Files delete write fence Each account's write blocks and its `FileDeleteCoordinator` live in an diff --git a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt index 375eb78a2..bf8d103fd 100644 --- a/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt +++ b/androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidNextcloudServices.kt @@ -19,6 +19,7 @@ import dev.obiente.nextcloudnative.app.AcquiredContractKind import dev.obiente.nextcloudnative.app.DeckAttachment import dev.obiente.nextcloudnative.app.DeckAttachmentOpenTarget import dev.obiente.nextcloudnative.app.DeckCardDraftKey +import dev.obiente.nextcloudnative.app.DirectoryCreation import dev.obiente.nextcloudnative.app.DurableUploadEnqueueResult import dev.obiente.nextcloudnative.app.DurableUploadScope import dev.obiente.nextcloudnative.app.DurableUploadStatus @@ -1397,8 +1398,12 @@ internal class AndroidNextcloudServices( check(response.status == 200 || response.status == 207 && fileFavoriteUpdateSucceeded(response.body)) { "The server did not confirm the favorite change." } - runCatching { fileReadCache.invalidate(NextcloudDocumentIds.accountKey(session), safePath) } - Unit + invalidateAfterConfirmedWrite(session, safePath) + } + + /** Invalidates cached content after a confirmed write of [paths] and reports whether that succeeded. */ + private fun invalidateAfterConfirmedWrite(session: NextcloudSession, vararg paths: String?) = NextcloudDocumentIds.accountKey(session).let { id -> + invalidateAfterConfirmedFileMutation(paths.filterNotNull(), { recordSupportDiagnosticForAccountIdentity(id, it) }) { fileReadCache.invalidate(id, it) } } override suspend fun loadFileOfflineAvailability( @@ -2565,7 +2570,7 @@ internal class AndroidNextcloudServices( userId: String, file: NextcloudFile, version: NextcloudFileVersion, - ): Unit = withContext(Dispatchers.IO) { + ) = withContext(Dispatchers.IO) { val specification = fileVersionRestoreRequest(userId, file, version) val response = request( method = specification.method, @@ -2579,7 +2584,7 @@ internal class AndroidNextcloudServices( client = noRedirectHttpClient, ) when (val result = classifyFileVersionRestoreHttpResponse(response.status)) { - FileVersionRestoreHttpResult.Restored -> Unit + FileVersionRestoreHttpResult.Restored -> invalidateAfterConfirmedWrite(session, file.path) is FileVersionRestoreHttpResult.Rejected -> error(result.message) } } @@ -2649,8 +2654,7 @@ internal class AndroidNextcloudServices( if (failure is CancellationException) throw failure null } - runCatching { fileReadCache.invalidate(NextcloudDocumentIds.accountKey(currentSession), path) } - SavedTextFile(etag, confirmation.created) + SavedTextFile(etag, confirmation.created, invalidateAfterConfirmedWrite(currentSession, path)) } } } @@ -2676,12 +2680,11 @@ internal class AndroidNextcloudServices( if (response.status == 412) return@withAndroidAuthenticatedFileMutation SavedTextFile(null, false) check(response.status in 200..299) { "Creating the text file failed (HTTP ${response.status})." } check(response.status == 201) { "The server did not confirm that a new text file was created." } - runCatching { fileReadCache.invalidate(NextcloudDocumentIds.accountKey(currentSession), path) } - SavedTextFile(response.etag, wasCreated = true) + SavedTextFile(response.etag, wasCreated = true, invalidateAfterConfirmedWrite(currentSession, path)) } } - override suspend fun createDirectoryIfAbsent(session: NextcloudSession, userId: String, path: String): Boolean = + override suspend fun createDirectoryIfAbsent(session: NextcloudSession, userId: String, path: String): DirectoryCreation = withContext(Dispatchers.IO) { withAndroidAuthenticatedFileMutation( accountMutationLeaseHeld, session, accountCredentials::loadSession, @@ -2694,11 +2697,10 @@ internal class AndroidNextcloudServices( maxResponseBytes = 64 * 1024, accountMutationSerialized = accountMutationSerialized, ) - if (response.status in setOf(405, 412)) return@withAndroidAuthenticatedFileMutation false + if (response.status in setOf(405, 412)) return@withAndroidAuthenticatedFileMutation DirectoryCreation(false) if (response.status !in 200..299) throw fileOperationException(response.status) check(response.status == 201) { "The server did not confirm that a new folder was created." } - runCatching { fileReadCache.invalidate(NextcloudDocumentIds.accountKey(currentSession), path) } - true + DirectoryCreation(wasCreated = true, invalidateAfterConfirmedWrite(currentSession, path)) } } @@ -2721,9 +2723,7 @@ internal class AndroidNextcloudServices( put("Overwrite", if (spec.overwrite) "T" else "F") } } - val accountId = NextcloudDocumentIds.accountKey(currentSession) - fun invalidateLocal() = invalidateAfterConfirmedFileMutation(listOfNotNull(spec.sourcePath, spec.destinationPath), - { event -> recordSupportDiagnosticForAccountIdentity(accountId, event) }) { path -> fileReadCache.invalidate(accountId, path) } + fun invalidateLocal() = invalidateAfterConfirmedWrite(currentSession, spec.sourcePath, spec.destinationPath) val response = dev.obiente.nextcloudnative.app.reportingUnknownFileMutationResult(::invalidateLocal) { request( method = spec.method, url = buildNextcloudFileUrl(currentSession.serverUrl, userId, spec.sourcePath), diff --git a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidShareUploadWriteFenceTest.kt b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidShareUploadWriteFenceTest.kt index 5d7642298..3e0d48539 100644 --- a/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidShareUploadWriteFenceTest.kt +++ b/androidApp/src/test/kotlin/dev/obiente/nextcloudnative/AndroidShareUploadWriteFenceTest.kt @@ -1,5 +1,6 @@ package dev.obiente.nextcloudnative +import dev.obiente.nextcloudnative.app.DirectoryCreation import dev.obiente.nextcloudnative.app.FileMutationNotSentException import dev.obiente.nextcloudnative.app.FileWriteBlock import dev.obiente.nextcloudnative.app.FileWriteBlockedException @@ -201,7 +202,7 @@ class AndroidShareUploadWriteFenceTest { "equals" -> proxy === args?.singleOrNull() "createDirectoryIfAbsent" -> { calls += "createDirectoryIfAbsent:${args!![2]}" - true + DirectoryCreation(wasCreated = true) } else -> error("Unexpected synthetic operation: ${method.name}") } diff --git a/changes/unreleased/477-desktop-write-local-follow-up.md b/changes/unreleased/477-desktop-write-local-follow-up.md new file mode 100644 index 000000000..a81a66cb8 --- /dev/null +++ b/changes/unreleased/477-desktop-write-local-follow-up.md @@ -0,0 +1,7 @@ +category: fix +issue: 477 +pull: 522 +platforms: linux, windows +user-facing: yes + +Creating folders and files, saving text, favorites, version restores, sync uploads, and Linux mount writes no longer wait for local file status after the server confirmed them, and incomplete local updates are reported. diff --git a/tools/kotlin-file-size-baseline.txt b/tools/kotlin-file-size-baseline.txt index e7bc08d60..d337394bf 100644 --- a/tools/kotlin-file-size-baseline.txt +++ b/tools/kotlin-file-size-baseline.txt @@ -21,10 +21,10 @@ ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NativeDeckBoardSurface. ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NativeDeckRelationDialogs.kt|1234 ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NativeDeckScreen.kt|1877 ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudMediaViewer.kt|1047 -ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt|11008 +ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt|11007 ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNotes.kt|1550 ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPhotoEditor.kt|808 -ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPlatform.kt|1716 +ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPlatform.kt|1711 ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/PhotoEditing.kt|847 ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/PhotoFolderBrowsing.kt|895 ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/PhotoTimelinePaging.kt|860 @@ -48,7 +48,7 @@ ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopAppUpdates.kt|9 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileReadCache.kt|1536 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileSyncEngine.kt|876 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileSyncRemoteTree.kt|857 -ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt|6115 +ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt|5987 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopVirtualRangeCache.kt|2759 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/LinuxVirtualFileSystem.kt|1697 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesJna.kt|1023 diff --git a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileOperations.kt b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileOperations.kt index 569f9a407..6a395421c 100644 --- a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileOperations.kt +++ b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileOperations.kt @@ -93,6 +93,22 @@ internal fun FileMutationLocalFollowUp.reportFor(done: String): String = when (t FileMutationLocalFollowUp.Failed -> "$done. Local file status could not be updated; refresh the folder if it looks out of date." } +/** A text file write the server confirmed. [localFollowUp] reports the local bookkeeping separately. */ +data class SavedTextFile( + val etag: String?, + val wasCreated: Boolean, + val localFollowUp: FileMutationLocalFollowUp = FileMutationLocalFollowUp.Completed, +) + +/** + * The result of creating a folder only when nothing exists at its path. [localFollowUp] reports + * the local bookkeeping after a confirmed creation; nothing changed when [wasCreated] is false. + */ +data class DirectoryCreation( + val wasCreated: Boolean, + val localFollowUp: FileMutationLocalFollowUp = FileMutationLocalFollowUp.Completed, +) + enum class NextcloudFileOperationError { AuthenticationRequired, PermissionDenied, diff --git a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileVersionHistorySection.kt b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileVersionHistorySection.kt index 588fbc9d0..1a64c7d72 100644 --- a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileVersionHistorySection.kt +++ b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileVersionHistorySection.kt @@ -260,8 +260,8 @@ internal fun FileVersionHistorySection( } is FileVersionConfirmation.Restore -> runCatching { services.restoreFileVersion(session, userId, file, version) - }.onSuccess { - actionMessage = "Version restored. Refreshing the current file..." + }.onSuccess { localFollowUp -> + actionMessage = fileVersionRestoredMessage(localFollowUp) runCatching { services.listFileVersions(session, userId, file) } .onSuccess { refreshed -> history = refreshed @@ -470,6 +470,11 @@ internal fun canRestoreFileVersion( requireRestorableFileVersion(file, version) }.isSuccess +/** What to say once the server restored a version; incomplete local bookkeeping is never hidden. */ +internal fun fileVersionRestoredMessage(localFollowUp: FileMutationLocalFollowUp): String = + if (localFollowUp == FileMutationLocalFollowUp.Completed) "Version restored. Refreshing the current file..." + else localFollowUp.reportFor("Version restored") + private fun ExternalFileHandoffResult.fileVersionActionMessage(): String = when (this) { is ExternalFileHandoffResult.Launched -> "A detached historical copy is ready in the platform export sheet." is ExternalFileHandoffResult.Cancelled -> "Export cancelled." diff --git a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt index 218c574c6..ece9cf02d 100644 --- a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt +++ b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt @@ -6703,13 +6703,13 @@ internal fun FilesScreen( scope.launch { filesRequest { services.setFileFavorite(session, requireNotNull(userId), file, favorite) - }.onSuccess { + }.onSuccess { localFollowUp -> favorites.succeeded(file.path) - mutationNotice = if (favorite) { + mutationNotice = localFollowUp.reportFor(if (favorite) { "${file.name} added to favorites" } else { "${file.name} removed from favorites" - } + }) }.onFailure { favorites.failed(file.path)?.let { previous -> files = files.withFavorite(file.path, previous) @@ -7129,10 +7129,9 @@ internal fun FilesScreen( filesRequest { when (plan.kind) { FileCreationKind.Folder -> { - check(services.createDirectoryIfAbsent(session, requireNotNull(userId), plan.path)) { - "An item named ${plan.name} already exists." - } - null + val created = services.createDirectoryIfAbsent(session, requireNotNull(userId), plan.path) + check(created.wasCreated) { "An item named ${plan.name} already exists." } + null to created.localFollowUp } FileCreationKind.Markdown, FileCreationKind.Text -> { val result = services.createTextFileIfAbsent( @@ -7157,13 +7156,13 @@ internal fun FilesScreen( hasPreview = false, etag = result.etag, permissions = "WDNVR", - ) + ) to result.localFollowUp } } - }.onSuccess { createdDocument -> + }.onSuccess { (createdDocument, localFollowUp) -> creationKind = null creationName = "" - mutationNotice = "Created ${plan.name}" + mutationNotice = localFollowUp.reportFor("Created ${plan.name}") if (createdDocument == null) { files = null loadAttempt += 1 diff --git a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPlatform.kt b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPlatform.kt index 1d2175128..d8e90c84d 100644 --- a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPlatform.kt +++ b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPlatform.kt @@ -187,11 +187,6 @@ data class NextcloudFileContent( val etag: String?, ) -data class SavedTextFile( - val etag: String?, - val wasCreated: Boolean, -) - enum class NextcloudApiMethod { GET, POST, @@ -785,13 +780,13 @@ interface NextcloudPlatformServices : NextcloudAccountCredentialServices, DeckCa scopePath: String = "", ): List = listFiles(session, userId, scopePath).filter(NextcloudFile::favorite) - /** Updates the server-backed favorite property and returns only after DAV confirms the write. */ + /** Returns only after DAV confirms the favorite write; local bookkeeping is reported separately. */ suspend fun setFileFavorite( session: NextcloudSession, userId: String, file: NextcloudFile, favorite: Boolean, - ) { + ): FileMutationLocalFollowUp { error("File favorites are not supported on this platform.") } @@ -1275,7 +1270,7 @@ interface NextcloudPlatformServices : NextcloudAccountCredentialServices, DeckCa userId: String, file: NextcloudFile, version: NextcloudFileVersion, - ) { + ): FileMutationLocalFollowUp { error("File version restoration is not supported on this platform.") } @@ -1316,7 +1311,7 @@ interface NextcloudPlatformServices : NextcloudAccountCredentialServices, DeckCa session: NextcloudSession, userId: String, path: String, - ): Boolean + ): DirectoryCreation suspend fun executeFileMutation( session: NextcloudSession, diff --git a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/TextEditorState.kt b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/TextEditorState.kt index e7eecd9cb..a6a6658ac 100644 --- a/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/TextEditorState.kt +++ b/ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/TextEditorState.kt @@ -148,7 +148,12 @@ internal class TextEditorState( originalText = submitted etag = saved.etag?.takeIf(String::isNotBlank) verificationRequired = etag == null - savedMessage = if (verificationRequired) "Saved. Verify the server version before saving again." else "Saved to Nextcloud" + // Verifying reads the server version, so only a plain success reports local status. + savedMessage = if (verificationRequired) { + "Saved. Verify the server version before saving again." + } else { + saved.localFollowUp.reportFor("Saved to Nextcloud") + } } catch (cancelled: CancellationException) { etag = null throw cancelled diff --git a/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/FileVersionHistoryTest.kt b/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/FileVersionHistoryTest.kt index ed723ce50..407cefce2 100644 --- a/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/FileVersionHistoryTest.kt +++ b/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/FileVersionHistoryTest.kt @@ -24,6 +24,22 @@ class FileVersionHistoryTest { ) } + @Test + fun `a restored version reports incomplete local status instead of plain success`() { + assertEquals( + "Version restored. Refreshing the current file...", + fileVersionRestoredMessage(FileMutationLocalFollowUp.Completed), + ) + assertEquals( + "Version restored. Local file status is still updating in the background.", + fileVersionRestoredMessage(FileMutationLocalFollowUp.StillRunning), + ) + assertEquals( + "Version restored. Local file status could not be updated; refresh the folder if it looks out of date.", + fileVersionRestoredMessage(FileMutationLocalFollowUp.Failed), + ) + } + @Test fun `plans official bounded versions DAV discovery`() { val request = fileVersionHistoryRequest("person@example.test", 42L) diff --git a/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/TextEditorStateTest.kt b/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/TextEditorStateTest.kt index 257464953..1428b4516 100644 --- a/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/TextEditorStateTest.kt +++ b/ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/TextEditorStateTest.kt @@ -53,6 +53,27 @@ class TextEditorStateTest { assertNull(editor.saveError) } + @Test + fun `a confirmed save reports incomplete local status instead of plain success`(): Unit = runBlocking { + for ((followUp, expected) in listOf( + FileMutationLocalFollowUp.Completed to "Saved to Nextcloud", + FileMutationLocalFollowUp.StillRunning to + "Saved to Nextcloud. Local file status is still updating in the background.", + FileMutationLocalFollowUp.Failed to + "Saved to Nextcloud. Local file status could not be updated; refresh the folder if it looks out of date.", + )) { + val editor = TextEditorState(MemoryStore(), { content("old", "v1") }, { _, _ -> SavedTextFile("v2", false, followUp) }) + editor.open() + editor.edit("draft") + + editor.save() + + assertEquals(expected, editor.savedMessage) + assertEquals("v2", editor.etag, "The server result stays authoritative.") + assertNull(editor.saveError) + } + } + @Test fun `a covering delete refuses the save without sending or losing edits`(): Unit = runBlocking { val store = MemoryStore() diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileMutationFollowUps.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileMutationFollowUps.kt index 4ebbc170a..939a751af 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileMutationFollowUps.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileMutationFollowUps.kt @@ -2,6 +2,7 @@ package dev.obiente.nextcloudnative.app import java.io.IOException import java.util.concurrent.atomic.AtomicReference +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope @@ -20,7 +21,8 @@ import kotlinx.coroutines.withTimeoutOrNull * One drain worker refreshes one remote path at a time in submission order. Pending refreshes are * coalesced per account and path, keeping only the newest refresh, and at most [maximumPending] * paths wait behind a stalled refresh. A path beyond that bound is reported as a local failure - * instead of growing the queue, both after a confirmed change and after an unknown result. + * instead of growing the queue, both after a confirmed change and after an unknown result. Only a + * background mutation that no caller waits for refreshes such a path on its own thread instead. */ internal class DesktopFileMutationFollowUps( private val scope: CoroutineScope, @@ -157,6 +159,31 @@ internal class DesktopFileMutationFollowUps( fun afterUnknownResult(accountId: String, paths: List, refresh: (path: String) -> Throwable?): Boolean = mutation(accountId, paths, refresh).onAcceptedOrUnknownResult() + /** + * Refreshes [paths] after a server-confirmed mutation that no caller waits for, such as a + * folder sync upload or a write through the Linux mount, without blocking the thread that + * reported it. When the queue is full, a path is refreshed on the calling thread instead: with + * no caller to tell, a dropped refresh would leave local state stale without anyone knowing, + * so a stalled queue slows these background writers down rather than losing their refreshes. + */ + fun afterBackgroundMutation(accountId: String, paths: List, refresh: (path: String) -> Throwable?) { + paths.distinct().forEach { path -> + val queued = submit(Key(accountId, path), { refresh(path) }, waiter = null, previous = null, reportDropped = false) + // A closed service runs nothing more; its account state is being torn down. + if (queued != null || !scope.isActive) return@forEach + recordOutcome(accountId, "inline", "FILE_MUTATION_LOCAL_FOLLOW_UP_QUEUE_FULL") + // Fault boundary for one blocking refresh, like the drain worker's. + val failure = try { + refresh(path) + } catch (cancelled: CancellationException) { + throw cancelled + } catch (failure: Exception) { + failure + } + failure?.let { recordOutcome(accountId, "failed", "FILE_MUTATION_LOCAL_FOLLOW_UP_FAILED", it) } + } + } + /** * Drops [accountId]'s queued refreshes at sign-out or account removal; their waiters and those * of its running refresh see a local failure. A running refresh cannot be interrupted, and its @@ -189,6 +216,7 @@ internal class DesktopFileMutationFollowUps( refresh: () -> Throwable?, waiter: Waiter?, previous: Pending?, + reportDropped: Boolean = true, ): Pending? { var finished: FileMutationLocalFollowUp? = null val entry = synchronized(lock) { @@ -210,7 +238,7 @@ internal class DesktopFileMutationFollowUps( entry } finished?.let { result -> waiter?.finish(result) } - if (entry == null) recordOutcome(key.accountId, "dropped", "FILE_MUTATION_LOCAL_FOLLOW_UP_DROPPED") + if (entry == null && reportDropped) recordOutcome(key.accountId, "dropped", "FILE_MUTATION_LOCAL_FOLLOW_UP_DROPPED") return entry } diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileWrites.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileWrites.kt new file mode 100644 index 000000000..689f810a3 --- /dev/null +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileWrites.kt @@ -0,0 +1,175 @@ +package dev.obiente.nextcloudnative.app + +/** One file write for the account a [DesktopFileWrites] is bound to. */ +internal class DesktopFileWriteRequest( + val method: String, + val url: String, + val headers: Map, + val body: ByteArray? = null, + val contentType: String? = null, + /** The success body bound; null keeps the transport's default. */ + val maximumResponseBytes: Long? = null, + /** False sends the request through the mutation executor that refuses redirects. */ + val followRedirects: Boolean = true, +) + +internal class DesktopFileWriteResponse(val status: Int, val etag: String?, val body: ByteArray) + +internal fun interface DesktopFileWriteTransport { + /** + * Sends [request] through the desktop mutation executor, which calls + * [onAcceptedOrUnknownResult] once the server accepted the request or the exchange was + * interrupted after it may have been sent, and raises + * [DesktopFileSyncAmbiguousMutationException] for the interrupted exchange. + */ + suspend fun send(request: DesktopFileWriteRequest, onAcceptedOrUnknownResult: () -> Unit): DesktopFileWriteResponse +} + +/** + * The desktop file writes of one account: folder and text file creation, text saves, favorites, + * version restores, and rename, move, copy, and delete. + * + * The server result is authoritative and reaches the caller promptly. Local cache and + * virtual-file bookkeeping runs on [followUps], queued once the server accepted a request or its + * result became unknown, and each confirmed result reports it as its local follow-up. + * [refreshLocalState] may wait on the virtual-file provider's monitor, which neither cancellation + * nor interruption can end, so it never runs on the caller's coroutine. + */ +internal class DesktopFileWrites( + private val accountId: String, + private val serverUrl: String, + private val userId: String, + private val followUps: DesktopFileMutationFollowUps, + private val transport: DesktopFileWriteTransport, + private val refreshLocalState: (path: String) -> Throwable?, + private val loadEtag: suspend (path: String) -> String? = { null }, + private val storeContent: (path: String, content: NextcloudFileContent) -> Unit = { _, _ -> }, + private val favoriteConfirmed: (multistatus: ByteArray) -> Boolean = { false }, +) { + suspend fun createDirectoryIfAbsent(path: String): DirectoryCreation { + val mutation = followUps.mutation(accountId, listOf(path), refreshLocalState) + val response = mutation.send( + DesktopFileWriteRequest("MKCOL", fileUrl(path), CREATE_ONLY_HEADERS, maximumResponseBytes = STATUS_RESPONSE_BYTES), + ) + if (response.status in setOf(405, 412)) return DirectoryCreation(wasCreated = false) + if (response.status !in 200..299) throw fileOperationException(response.status) + check(response.status == 201) { "The server did not confirm that a new folder was created." } + return DirectoryCreation(wasCreated = true, mutation.afterConfirmedMutation()) + } + + suspend fun createTextFileIfAbsent(path: String, text: String): SavedTextFile { + val utf8 = text.encodeToByteArray() + require(utf8.size.toLong() <= MAX_EDITABLE_TEXT_BYTES) { + "Text files larger than ${MAX_EDITABLE_TEXT_BYTES / (1024 * 1024)} MiB cannot be created in the app." + } + val mutation = followUps.mutation(accountId, listOf(path), refreshLocalState) + val response = mutation.send(DesktopFileWriteRequest("PUT", fileUrl(path), CREATE_ONLY_HEADERS, utf8, TEXT_CONTENT_TYPE)) + if (response.status == 412) return SavedTextFile(etag = null, wasCreated = false) + check(response.status in 200..299) { "Creating the text file failed (HTTP ${response.status})." } + check(response.status == 201) { "The server did not confirm that a new text file was created." } + val followUp = mutation.afterConfirmedMutation() + response.etag?.let { etag -> prefillReadCache(path, NextcloudFileContent(utf8, TEXT_CONTENT_TYPE, etag)) } + return SavedTextFile(response.etag, wasCreated = true, followUp) + } + + suspend fun saveTextFile(path: String, text: String, expectedEtag: String): SavedTextFile { + val specification = textFileDavSaveRequest(text, expectedEtag) + val mutation = followUps.mutation(accountId, listOf(path), refreshLocalState) + val response = mutation.send( + DesktopFileWriteRequest("PUT", fileUrl(path), specification.headers, specification.body, specification.contentType), + ) + val confirmation = confirmTextFileDavSave(response.status) + val etag = response.etag ?: runCatchingPreservingCancellation { loadEtag(path) }.getOrNull() + val followUp = mutation.afterConfirmedMutation() + etag?.let { prefillReadCache(path, NextcloudFileContent(specification.body, specification.contentType, it)) } + return SavedTextFile(etag, confirmation.created, followUp) + } + + suspend fun setFileFavorite(file: NextcloudFile, favorite: Boolean): FileMutationLocalFollowUp { + val safePath = requireSafeFilePath(file.path, allowRoot = false) + val expectedEtag = file.etag?.trim().orEmpty() + require(expectedEtag.isNotEmpty()) { "Refresh the folder before changing favorites." } + val condition = FileWebDavMutationSpec( + method = "PROPPATCH", + sourcePath = safePath, + destinationPath = null, + expectedEtag = expectedEtag, + sourceIsDirectory = file.isDirectory, + overwrite = false, + ).conflictConditionHeaders() + val mutation = followUps.mutation(accountId, listOf(safePath), refreshLocalState) + val response = mutation.send( + DesktopFileWriteRequest( + "PROPPATCH", + fileUrl(safePath), + mapOf("Accept" to "application/xml") + condition, + buildFileFavoritePropPatch(favorite).encodeToByteArray(), + "application/xml; charset=utf-8", + STATUS_RESPONSE_BYTES, + ), + ) + if (response.status !in 200..299) throw fileOperationException(response.status) + check(response.status == 200 || response.status == 207 && favoriteConfirmed(response.body)) { + "The server did not confirm the favorite change." + } + return mutation.afterConfirmedMutation() + } + + suspend fun restoreFileVersion(file: NextcloudFile, version: NextcloudFileVersion): FileMutationLocalFollowUp { + val specification = fileVersionRestoreRequest(userId, file, version) + val mutation = followUps.mutation(accountId, listOf(file.path), refreshLocalState) + val response = mutation.send( + DesktopFileWriteRequest( + specification.method, + serverUrl + specification.relativePath, + specification.headers + mapOf( + "Accept" to "*/*", + "Destination" to serverUrl + specification.destinationRelativePath, + ), + maximumResponseBytes = specification.maximumResponseBytes, + followRedirects = false, + ), + ) + return when (val result = classifyFileVersionRestoreHttpResponse(response.status)) { + FileVersionRestoreHttpResult.Restored -> mutation.afterConfirmedMutation() + is FileVersionRestoreHttpResult.Rejected -> error(result.message) + } + } + + suspend fun executeFileMutation(mutation: NextcloudFileMutation): NextcloudFileMutationResult { + val spec = mutation.toWebDavMutationSpec() + val headers = buildMap { + put("Accept", "*/*") + putAll(spec.conflictConditionHeaders()) + spec.destinationPath?.let { destinationPath -> + put("Destination", fileUrl(destinationPath)) + put("Overwrite", if (spec.overwrite) "T" else "F") + } + } + val followUp = followUps.mutation(accountId, listOfNotNull(spec.sourcePath, spec.destinationPath), refreshLocalState) + val response = followUp.send( + DesktopFileWriteRequest(spec.method, fileUrl(spec.sourcePath), headers, maximumResponseBytes = STATUS_RESPONSE_BYTES), + ) + if (response.status !in 200..299) { + val gone = if (response.status == 404) followUp.afterConfirmedMutation() else null + throw fileOperationException(response.status, spec.sourceIsDirectory, gone) + } + return NextcloudFileMutationResult(spec.destinationPath, response.etag, followUp.afterConfirmedMutation()) + } + + private suspend fun DesktopFileMutationFollowUps.Mutation.send(request: DesktopFileWriteRequest): DesktopFileWriteResponse = + reconcilingUnknownResult { onAcceptedOrUnknownResult -> transport.send(request, onAcceptedOrUnknownResult) } + + private fun fileUrl(path: String): String = buildNextcloudFileUrl(serverUrl, userId, path) + + /** Best effort: a read-cache miss only costs a later download of what the server confirmed. */ + private fun prefillReadCache(path: String, content: NextcloudFileContent) { + runCatching { storeContent(path, content) } + } + + private companion object { + val CREATE_ONLY_HEADERS = mapOf("Accept" to "*/*", "If-None-Match" to "*") + const val TEXT_CONTENT_TYPE = "text/plain; charset=utf-8" + const val STATUS_RESPONSE_BYTES = 64L * 1024L + } +} diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt index 948f8edd2..d264b1896 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt @@ -1265,6 +1265,20 @@ class DesktopNextcloudServices( return failure } + /** + * Queues [refreshRetainedFoldersAfterMutation] for a server-confirmed background mutation, such + * as a folder sync upload or a Linux mount write, so its thread never waits on the provider lock. + */ + private fun refreshAfterBackgroundMutation( + session: NextcloudSession, + userId: String, + accountId: String, + path: String, + cacheProducer: DesktopFileReadCacheProducer?, + ) = fileMutationFollowUps.afterBackgroundMutation(accountId, listOf(path)) { changed -> + refreshRetainedFoldersAfterMutation(session, userId, accountId, changed, cacheProducer) + } + private fun refreshRetainedFoldersAfterRemoteListing( session: NextcloudSession, userId: String, @@ -1327,7 +1341,7 @@ class DesktopNextcloudServices( minimumFreeSpaceBytes = { fileReadCache.loadPolicy().minimumFreeSpaceBytes }, onRemoteMutationCommitted = { session, userId, path -> val (accountId, cacheProducer) = fileReadCache.producerFor(session) - refreshRetainedFoldersAfterMutation(session, userId, accountId, path, cacheProducer) + refreshAfterBackgroundMutation(session, userId, accountId, path, cacheProducer) }, ) private val accountSyncPairCleanupJournal = DesktopAccountSyncPairCleanupJournal( @@ -1861,7 +1875,7 @@ class DesktopNextcloudServices( }, ), afterMutationInvalidated = { path -> - refreshRetainedFoldersAfterMutation(session, userId, accountId, path, cacheProducer) + refreshAfterBackgroundMutation(session, userId, accountId, path, cacheProducer) }, ) metadataBackendReference = metadataBackend @@ -4174,44 +4188,7 @@ class DesktopNextcloudServices( userId: String, file: NextcloudFile, favorite: Boolean, - ) = withContext(Dispatchers.IO) { - val safePath = requireSafeFilePath(file.path, allowRoot = false) - val expectedEtag = file.etag?.trim().orEmpty() - require(expectedEtag.isNotEmpty()) { "Refresh the folder before changing favorites." } - val headers = buildMap { - put("Accept", "application/xml") - putAll( - FileWebDavMutationSpec( - method = "PROPPATCH", - sourcePath = safePath, - destinationPath = null, - expectedEtag = expectedEtag, - sourceIsDirectory = file.isDirectory, - overwrite = false, - ).conflictConditionHeaders(), - ) - } - val (accountId, cacheProducer) = fileReadCache.producerFor(session) - fun refreshMetadata() { - runCatching { refreshRetainedFoldersAfterMutation(session, userId, accountId, safePath, cacheProducer) } - } - val response = request( - method = "PROPPATCH", - url = buildNextcloudFileUrl(session.serverUrl, userId, safePath), - session = session, - body = buildFileFavoritePropPatch(favorite), - contentType = "application/xml; charset=utf-8", - headers = headers, - maxResponseBytes = 64 * 1024, - mutationExecutor = fileMutationHttpExecutor, - onAmbiguousMutationResult = ::refreshMetadata, - ) - if (response.status !in 200..299) throw fileOperationException(response.status) - check(response.status == 200 || response.status == 207 && fileFavoriteUpdateSucceeded(response.body)) { - "The server did not confirm the favorite change." - } - refreshMetadata() - } + ): FileMutationLocalFollowUp = withContext(Dispatchers.IO) { fileWrites(session, userId).setFileFavorite(file, favorite) } override suspend fun listMedia(session: NextcloudSession, userId: String): List = withContext(Dispatchers.IO) { @@ -4599,33 +4576,7 @@ class DesktopNextcloudServices( userId: String, file: NextcloudFile, version: NextcloudFileVersion, - ): Unit = withContext(Dispatchers.IO) { - val specification = fileVersionRestoreRequest(userId, file, version) - val (accountId, cacheProducer) = fileReadCache.producerFor(session) - fun queueAffectedMetadataRefresh() = - refreshRetainedFoldersAfterMutation(session, userId, accountId, file.path, cacheProducer) - val response = request( - method = specification.method, - url = session.serverUrl + specification.relativePath, - session = session, - headers = specification.headers + mapOf( - "Accept" to "*/*", - "Destination" to session.serverUrl + specification.destinationRelativePath, - ), - maxResponseBytes = specification.maximumResponseBytes, - client = noRedirectHttpClient, - mutationExecutor = noRedirectFileMutationHttpExecutor, - onAmbiguousMutationResult = ::queueAffectedMetadataRefresh, - ) - when (val result = classifyFileVersionRestoreHttpResponse(response.status)) { - FileVersionRestoreHttpResult.Restored -> { - runCatching { - refreshRetainedFoldersAfterMutation(session, userId, accountId, file.path, cacheProducer) - } - } - is FileVersionRestoreHttpResult.Rejected -> error(result.message) - } - } + ): FileMutationLocalFollowUp = withContext(Dispatchers.IO) { fileWrites(session, userId).restoreFileVersion(file, version) } override suspend fun handoffFileVersionToExternalApp( session: NextcloudSession, @@ -4674,130 +4625,51 @@ class DesktopNextcloudServices( path: String, text: String, expectedEtag: String, - ): SavedTextFile = withContext(Dispatchers.IO) { - val specification = textFileDavSaveRequest(text, expectedEtag) - val (accountId, cacheProducer) = fileReadCache.producerFor(session) - fun queueAffectedMetadataRefresh() = - refreshRetainedFoldersAfterMutation(session, userId, accountId, path, cacheProducer) - val response = request( - "PUT", - buildNextcloudFileUrl(session.serverUrl, userId, path), - session, - rawBody = specification.body, - contentType = specification.contentType, - headers = specification.headers, - mutationExecutor = fileMutationHttpExecutor, - onAmbiguousMutationResult = ::queueAffectedMetadataRefresh, - ) - val confirmation = confirmTextFileDavSave(response.status) - val etag = response.etag ?: - runCatchingPreservingCancellation { loadFileEtag(session, userId, path) }.getOrNull() - runCatching { - refreshRetainedFoldersAfterMutation(session, userId, accountId, path, cacheProducer) - etag?.let { - fileReadCache.storeContent( - accountId, path, - NextcloudFileContent(specification.body, specification.contentType, it), - cacheProducer = cacheProducer, - ) - } - } - SavedTextFile(etag, confirmation.created) - } + ): SavedTextFile = withContext(Dispatchers.IO) { fileWrites(session, userId).saveTextFile(path, text, expectedEtag) } override suspend fun createTextFileIfAbsent( session: NextcloudSession, userId: String, path: String, text: String, - ): SavedTextFile = withContext(Dispatchers.IO) { - val utf8 = text.toByteArray(StandardCharsets.UTF_8) - require(utf8.size.toLong() <= MAX_EDITABLE_TEXT_BYTES) { - "Text files larger than ${MAX_EDITABLE_TEXT_BYTES / (1024 * 1024)} MiB cannot be created in the app." - } - val (accountId, cacheProducer) = fileReadCache.producerFor(session) - fun queueAffectedMetadataRefresh() = - refreshRetainedFoldersAfterMutation(session, userId, accountId, path, cacheProducer) - val response = request( - "PUT", - buildNextcloudFileUrl(session.serverUrl, userId, path), - session, - rawBody = utf8, - contentType = "text/plain; charset=utf-8", - headers = mapOf("Accept" to "*/*", "If-None-Match" to "*"), - mutationExecutor = fileMutationHttpExecutor, - onAmbiguousMutationResult = ::queueAffectedMetadataRefresh, - ) - if (response.status == 412) return@withContext SavedTextFile(etag = null, wasCreated = false) - check(response.status in 200..299) { "Creating the text file failed (HTTP ${response.status})." } - check(response.status == 201) { "The server did not confirm that a new text file was created." } - runCatching { - refreshRetainedFoldersAfterMutation(session, userId, accountId, path, cacheProducer) - response.etag?.let { - fileReadCache.storeContent( - accountId, path, - NextcloudFileContent(utf8, "text/plain; charset=utf-8", it), - cacheProducer = cacheProducer, - ) - } - } - SavedTextFile(response.etag, wasCreated = true) - } + ): SavedTextFile = withContext(Dispatchers.IO) { fileWrites(session, userId).createTextFileIfAbsent(path, text) } override suspend fun createDirectoryIfAbsent( session: NextcloudSession, userId: String, path: String, - ): Boolean = withContext(Dispatchers.IO) { - val (accountId, cacheProducer) = fileReadCache.producerFor(session) - fun queueAffectedMetadataRefresh() = - refreshRetainedFoldersAfterMutation(session, userId, accountId, path, cacheProducer) - val response = request( - method = "MKCOL", - url = buildNextcloudFileUrl(session.serverUrl, userId, path), - session = session, - headers = mapOf("Accept" to "*/*", "If-None-Match" to "*"), - maxResponseBytes = 64 * 1024, - mutationExecutor = fileMutationHttpExecutor, - onAmbiguousMutationResult = ::queueAffectedMetadataRefresh, - ) - if (response.status in setOf(405, 412)) return@withContext false - if (response.status !in 200..299) throw fileOperationException(response.status) - check(response.status == 201) { "The server did not confirm that a new folder was created." } - runCatching { - refreshRetainedFoldersAfterMutation(session, userId, accountId, path, cacheProducer) - } - true - } + ): DirectoryCreation = withContext(Dispatchers.IO) { fileWrites(session, userId).createDirectoryIfAbsent(path) } override suspend fun executeFileMutation( session: NextcloudSession, userId: String, mutation: NextcloudFileMutation, - ): NextcloudFileMutationResult = withContext(Dispatchers.IO) { - val spec = mutation.toWebDavMutationSpec() - val headers = buildMap { - put("Accept", "*/*") - putAll(spec.conflictConditionHeaders()) - spec.destinationPath?.let { destinationPath -> - put("Destination", buildNextcloudFileUrl(session.serverUrl, userId, destinationPath)) - put("Overwrite", if (spec.overwrite) "T" else "F") - } - } + ): NextcloudFileMutationResult = withContext(Dispatchers.IO) { fileWrites(session, userId).executeFileMutation(mutation) } + + /** File writes of [session]'s account; their local bookkeeping runs on [fileMutationFollowUps]. */ + private fun fileWrites(session: NextcloudSession, userId: String): DesktopFileWrites { val (accountId, cacheProducer) = fileReadCache.producerFor(session) - fun refreshLocalState(path: String) = refreshRetainedFoldersAfterMutation(session, userId, accountId, path, cacheProducer) - val followUp = fileMutationFollowUps.mutation(accountId, listOfNotNull(spec.sourcePath, spec.destinationPath), ::refreshLocalState) - val response = followUp.reconcilingUnknownResult { onUnknownResult -> request( - method = spec.method, - url = buildNextcloudFileUrl(session.serverUrl, userId, spec.sourcePath), - session = session, - headers = headers, - maxResponseBytes = 64 * 1024, - mutationExecutor = fileMutationHttpExecutor, - onAmbiguousMutationResult = onUnknownResult, - ) } - if (response.status !in 200..299) throw fileOperationException(response.status, spec.sourceIsDirectory, if (response.status == 404) followUp.afterConfirmedMutation() else null) - NextcloudFileMutationResult(spec.destinationPath, response.etag, followUp.afterConfirmedMutation()) + return DesktopFileWrites( + accountId, session.serverUrl, userId, fileMutationFollowUps, + transport = { write, onAcceptedOrUnknownResult -> + val response = request( + method = write.method, + url = write.url, + session = session, + contentType = write.contentType, + headers = write.headers, + rawBody = write.body, + maxResponseBytes = write.maximumResponseBytes ?: MAX_API_RESPONSE_BYTES, + mutationExecutor = if (write.followRedirects) fileMutationHttpExecutor else noRedirectFileMutationHttpExecutor, + onAmbiguousMutationResult = onAcceptedOrUnknownResult, + ) + DesktopFileWriteResponse(response.status, response.etag, response.body) + }, + refreshLocalState = { path -> refreshRetainedFoldersAfterMutation(session, userId, accountId, path, cacheProducer) }, + loadEtag = { path -> loadFileEtag(session, userId, path) }, + storeContent = { path, content -> fileReadCache.storeContent(accountId, path, content, cacheProducer = cacheProducer) }, + favoriteConfirmed = ::fileFavoriteUpdateSucceeded, + ) } override suspend fun executeNextcloudApi( diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopFileMutationFollowUpsTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopFileMutationFollowUpsTest.kt index 7e2115fbb..988c35196 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopFileMutationFollowUpsTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopFileMutationFollowUpsTest.kt @@ -3,6 +3,7 @@ package dev.obiente.nextcloudnative.app import java.util.Collections import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit +import kotlin.concurrent.thread import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob @@ -545,6 +546,69 @@ class DesktopFileMutationFollowUpsTest { ) } + @Test + fun `a background mutation never waits for a stalled refresh`() { + val followUps = followUps() + val release = latch() + val returned = CountDownLatch(1) + val finished = CountDownLatch(1) + + thread(isDaemon = true) { + followUps.afterBackgroundMutation("account", listOf("uploaded")) { + release.awaitUninterruptibly() + finished.countDown() + null + } + returned.countDown() + } + + assertTrue(returned.await(5, TimeUnit.SECONDS), "The sync or mount thread must not wait for the refresh.") + assertEquals(1L, finished.count) + release.countDown() + assertTrue(finished.await(5, TimeUnit.SECONDS), "The refresh still runs in the background.") + assertTrue(diagnostics.isEmpty()) + } + + @Test + fun `a full queue refreshes a background mutation on its own thread instead of dropping it`(): Unit = runBlocking { + val followUps = followUps(maximumPending = 1) + val release = latch() + val started = CountDownLatch(1) + followUps.afterUnknownResult("account", listOf("stalled")) { + started.countDown() + release.awaitUninterruptibly() + null + } + assertTrue(started.await(5, TimeUnit.SECONDS)) + followUps.afterBackgroundMutation("account", listOf("queued")) { null } + val caller = Thread.currentThread() + var refreshedOn: Thread? = null + + followUps.afterBackgroundMutation("account", listOf("overflow")) { + refreshedOn = Thread.currentThread() + IllegalStateException("cache") + } + + assertSame(caller, refreshedOn, "With no caller to report a drop to, the refresh is delayed, not lost.") + assertEquals(1, followUps.pendingCount) + assertEquals(listOf("inline", "failed"), diagnostics.map { it.second.outcome }) + assertTrue(diagnostics.none { it.second.outcome == "dropped" }) + } + + @Test + fun `a background mutation after service teardown runs nothing`() { + val followUps = followUps() + scope.cancel() + var ran = false + + followUps.afterBackgroundMutation("account", listOf("late")) { + ran = true + null + } + + assertFalse(ran) + } + private fun followUps(waitMillis: Long = 5_000L, maximumPending: Int = MAX_PENDING_FILE_MUTATION_FOLLOW_UPS) = DesktopFileMutationFollowUps( scope = scope, diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopFileWritesTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopFileWritesTest.kt new file mode 100644 index 000000000..db9d5316a --- /dev/null +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopFileWritesTest.kt @@ -0,0 +1,240 @@ +package dev.obiente.nextcloudnative.app + +import java.io.IOException +import java.util.Collections +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import kotlin.concurrent.thread +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.async +import kotlinx.coroutines.cancel +import kotlinx.coroutines.cancelAndJoin +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout +import kotlin.test.AfterTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertIs +import kotlin.test.assertTrue + +/** + * Every desktop file write returns its server result while local bookkeeping is stuck behind the + * virtual-file provider's monitor. [providerLock] stands in for that monitor: a test thread holds + * it, and each refresh waits for it exactly like the production refresh. + */ +class DesktopFileWritesTest { + private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO) + private val providerLock = Any() + private val entered = Collections.synchronizedList(mutableListOf()) + private val refreshed = Collections.synchronizedList(mutableListOf()) + private val releases = Collections.synchronizedList(mutableListOf()) + + @AfterTest + fun tearDown() { + releases.forEach(CountDownLatch::countDown) + scope.cancel() + } + + @Test + fun `every routed write returns its server result while the provider lock is held`(): Unit = runBlocking { + for (write in routedWrites) { + val followUps = followUps(waitMillis = 50L) + val writes = writes(followUps, respondingTo(write.status)) + val release = holdProviderLock() + + val followUp = withTimeout(5_000L) { write.run(writes) } + + assertEquals(FileMutationLocalFollowUp.StillRunning, followUp, write.name) + assertEquals( + "Done. Local file status is still updating in the background.", + followUp.reportFor("Done"), + "${write.name} is not reported as plain success.", + ) + assertTrue(refreshed.none { it in write.paths }, "${write.name} cannot refresh while the lock is held.") + release.countDown() + withTimeout(5_000L) { while (!refreshed.containsAll(write.paths)) delay(10L) } + refreshed.clear() + } + } + + @Test + fun `an unknown result is reported without waiting for the stuck refresh`(): Unit = runBlocking { + val followUps = followUps(waitMillis = 50L) + val writes = writes(followUps) { _, onAcceptedOrUnknownResult -> + onAcceptedOrUnknownResult() + throw DesktopFileSyncAmbiguousMutationException(IOException("reset")) + } + holdProviderLock() + + val unknown = assertFailsWith { + withTimeout(5_000L) { writes.createDirectoryIfAbsent("Docs/New") } + } + + assertEquals(FileMutationLocalFollowUp.StillRunning, unknown.localFollowUp) + } + + @Test + fun `a write the server did not apply queues no local refresh`(): Unit = runBlocking { + val followUps = followUps() + val writes = writes(followUps, respondingTo(412)) + + assertEquals(DirectoryCreation(wasCreated = false), writes.createDirectoryIfAbsent("Docs/New")) + assertEquals(SavedTextFile(etag = null, wasCreated = false), writes.createTextFileIfAbsent("Docs/new.md", "")) + assertEquals(0, followUps.pendingCount) + assertTrue(refreshed.isEmpty()) + } + + @Test + fun `cancelling the caller keeps the queued refresh and is not reported as an error`(): Unit = runBlocking { + for (write in routedWrites) { + val followUps = followUps(waitMillis = 60_000L) + val sent = CountDownLatch(1) + val writes = writes(followUps) { request, onAcceptedOrUnknownResult -> + respondingTo(write.status).send(request, onAcceptedOrUnknownResult).also { sent.countDown() } + } + val release = holdProviderLock() + var failure: Throwable? = null + val caller = scope.launch { + try { + write.run(writes) + } catch (thrown: Throwable) { + failure = thrown + throw thrown + } + } + assertTrue(sent.await(5, TimeUnit.SECONDS), write.name) + + withTimeout(5_000L) { caller.cancelAndJoin() } + + assertIs(failure, "${write.name} reports cancellation, not a failure: $failure") + release.countDown() + withTimeout(5_000L) { while (!refreshed.containsAll(write.paths)) delay(10L) } + refreshed.clear() + } + } + + @Test + fun `account abandonment fails waiting writes and ignores the stale completion`(): Unit = runBlocking { + val followUps = followUps(waitMillis = 60_000L) + val writes = writes(followUps, respondingTo(201)) + val release = holdProviderLock() + val running = async { writes.createDirectoryIfAbsent("Docs/Running") } + withTimeout(5_000L) { while ("Docs/Running" !in entered) delay(10L) } + val queued = async { writes.createTextFileIfAbsent("Docs/queued.md", "") } + withTimeout(5_000L) { while (followUps.pendingCount < 1) delay(10L) } + + followUps.abandonAccount(ACCOUNT) + + assertEquals(FileMutationLocalFollowUp.Failed, withTimeout(5_000L) { running.await() }.localFollowUp) + assertEquals(FileMutationLocalFollowUp.Failed, withTimeout(5_000L) { queued.await() }.localFollowUp) + release.countDown() + withTimeout(5_000L) { while ("Docs/Running" !in refreshed) delay(10L) } + assertFalse("Docs/queued.md" in refreshed, "An abandoned refresh never runs: $refreshed") + assertEquals(0, followUps.pendingCount) + } + + @Test + fun `a full queue reports that the folder must be refreshed`(): Unit = runBlocking { + val followUps = followUps(waitMillis = 50L, maximumPending = 1) + val writes = writes(followUps, respondingTo(201)) + holdProviderLock() + assertEquals(FileMutationLocalFollowUp.StillRunning, writes.createDirectoryIfAbsent("Docs/Running").localFollowUp) + withTimeout(5_000L) { while ("Docs/Running" !in entered) delay(10L) } + assertEquals(FileMutationLocalFollowUp.StillRunning, writes.createDirectoryIfAbsent("Docs/Queued").localFollowUp) + + val dropped = withTimeout(5_000L) { writes.createDirectoryIfAbsent("Docs/Dropped") } + + assertEquals(DirectoryCreation(wasCreated = true, FileMutationLocalFollowUp.Failed), dropped) + assertEquals( + "Created Dropped. Local file status could not be updated; refresh the folder if it looks out of date.", + dropped.localFollowUp.reportFor("Created Dropped"), + ) + val unknownWrites = writes(followUps) { _, onAcceptedOrUnknownResult -> + onAcceptedOrUnknownResult() + throw DesktopFileSyncAmbiguousMutationException(IOException("reset")) + } + val unknown = assertFailsWith { + withTimeout(5_000L) { unknownWrites.createDirectoryIfAbsent("Docs/Unknown") } + } + assertEquals(FileMutationLocalFollowUp.Failed, unknown.localFollowUp) + } + + private class RoutedWrite( + val name: String, + val status: Int, + val paths: List, + val run: suspend (DesktopFileWrites) -> FileMutationLocalFollowUp, + ) + + private val notes = NextcloudFile( + path = "Docs/notes.txt", + name = "notes.txt", + isDirectory = false, + mimeType = "text/plain", + size = 4L, + lastModified = null, + fileId = 7L, + hasPreview = false, + etag = "\"e1\"", + permissions = "RGDNVW", + ) + + private val routedWrites = listOf( + RoutedWrite("create folder", 201, listOf("Docs/New")) { it.createDirectoryIfAbsent("Docs/New").localFollowUp }, + RoutedWrite("create text file", 201, listOf("Docs/new.md")) { it.createTextFileIfAbsent("Docs/new.md", "").localFollowUp }, + RoutedWrite("save text file", 204, listOf(notes.path)) { it.saveTextFile(notes.path, "text", "\"e1\"").localFollowUp }, + RoutedWrite("favorite", 200, listOf(notes.path)) { it.setFileFavorite(notes, favorite = true) }, + RoutedWrite("restore version", 201, listOf(notes.path)) { + it.restoreFileVersion(notes, NextcloudFileVersion(7L, "1700000000", 1L, "Older", "e0", null, null)) + }, + RoutedWrite("rename", 201, listOf(notes.path, "Docs/renamed.txt")) { + it.executeFileMutation(NextcloudFileMutation.Rename(notes.path, "renamed.txt", "\"e1\"")).localFollowUp + }, + ) + + private fun respondingTo(status: Int) = DesktopFileWriteTransport { _, onAcceptedOrUnknownResult -> + if (status in 200..299) onAcceptedOrUnknownResult() + DesktopFileWriteResponse(status, etag = "\"e2\"", body = byteArrayOf()) + } + + private fun writes(followUps: DesktopFileMutationFollowUps, transport: DesktopFileWriteTransport) = DesktopFileWrites( + accountId = ACCOUNT, + serverUrl = "https://cloud.example.test", + userId = "alice", + followUps = followUps, + transport = transport, + refreshLocalState = { path -> + entered += path + synchronized(providerLock) { refreshed += path } + null + }, + ) + + private fun followUps(waitMillis: Long = 5_000L, maximumPending: Int = MAX_PENDING_FILE_MUTATION_FOLLOW_UPS) = + DesktopFileMutationFollowUps(scope, completionWaitMillis = waitMillis, maximumPending = maximumPending) + + /** Holds [providerLock] on another thread, as a stalled virtual-file provider call would. */ + private fun holdProviderLock(): CountDownLatch { + val release = CountDownLatch(1).also(releases::add) + val held = CountDownLatch(1) + thread(isDaemon = true, name = "stalled-virtual-file-provider") { + synchronized(providerLock) { + held.countDown() + release.await(5, TimeUnit.SECONDS) + } + } + assertTrue(held.await(5, TimeUnit.SECONDS), "The provider lock was not taken.") + return release + } + + private companion object { + const val ACCOUNT = "account-digest" + } +} diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/FileWriteFenceRegistryTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/FileWriteFenceRegistryTest.kt index 01593c94e..c15e8c994 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/FileWriteFenceRegistryTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/FileWriteFenceRegistryTest.kt @@ -223,7 +223,7 @@ class FileWriteFenceRegistryTest { "equals" -> proxy === args?.singleOrNull() "createDirectoryIfAbsent" -> { calls += "createDirectoryIfAbsent:${args!![2]}" - true + DirectoryCreation(wasCreated = true) } else -> error("Unexpected synthetic operation: ${method.name}") } diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/FileWriteFenceSurfacesTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/FileWriteFenceSurfacesTest.kt index ae7efcee7..65eec393d 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/FileWriteFenceSurfacesTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/FileWriteFenceSurfacesTest.kt @@ -86,11 +86,11 @@ class FileWriteFenceSurfacesTest { ) "createDirectoryIfAbsent" -> { calls += "createDirectoryIfAbsent:${args!![2]}" - true + DirectoryCreation(wasCreated = true) } "restoreFileVersion" -> { calls += "restoreFileVersion:${(args!![2] as NextcloudFile).path}" - Unit + FileMutationLocalFollowUp.Completed } else -> error("Unexpected synthetic operation: ${method.name}") } diff --git a/website/public/screenshots/capture-manifest.json b/website/public/screenshots/capture-manifest.json index 583bccc62..049edb69a 100644 --- a/website/public/screenshots/capture-manifest.json +++ b/website/public/screenshots/capture-manifest.json @@ -626,7 +626,7 @@ "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileOfflineCenterScreen.kt": "8c6a8c48ddcbae74bf47035c95ed04cde36fcb73bf7b538af2d16125b42fe920", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileOfflineQueue.kt": "51fced0946f36654515a8a3b2a827f564e61e7626d4fdd5f194139d6aeaddaf9", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileOfflineQueueSnapshot.kt": "1fb930db8f65e0e410af6eaacde0c4f3d071f4115c39f78a4f49a9532b9f7961", - "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileOperations.kt": "74fb975ce99ffa9319a8cbee16219ad731d014b73c7a9591daad727d6ab02811", + "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileOperations.kt": "d05afa8f485dee5ba99f7e5427a0283a32a3ba0edd2d989fc51c4a0744190e4a", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileShareCreationFields.kt": "ae14656030b9c107907408209d07ed352a8307d577d02f912ee2f9ab8bbe330d", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileShareDialog.kt": "e26cee24080b124efb1016fc05168e578965e4265a85c221626d954e8abc655a", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileShareEmailAddress.kt": "48ac473d694955b0591b33224f782717ff1b6cfde92b20e50f566bd830101c64", @@ -655,7 +655,7 @@ "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileSyncUploadOwnership.kt": "b3dbc9fa663592e783991faaa9eea0b43bf368886b6ecc9834a6ffab019b6f1a", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileTransferDialog.kt": "06d4ca3ab5a19d25fe9bf98a02aee7142bcfe2299edfd831398411863a0fd230", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileVersionHistory.kt": "7d01527acc00eafe092885d19f318734263597b4c2c5c09cf39159d174a371dd", - "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileVersionHistorySection.kt": "f92fbf60947be94f5d58da7c0be429ed59c2e8344341ff9a19fa7dc6ace2bf58", + "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileVersionHistorySection.kt": "90e3dfa581893cd422a42122f6ff798cfb1c8290e3411762b1aa0f01d3b48f1a", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileVersionRestoreHttpPolicy.kt": "2887700cfc59c51ebcc4ec87575b037c74bd7e47e33568f46ace10bf2a1fc2c5", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileWriteFence.kt": "9b2035cbb70f1046ee61ae34a415fb1e245de7ad041925613fce27652cc7fb4f", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FilesRequestState.kt": "0e88380b24d51e1b9fb12931b471f9cc4c5e544714a9c0964eaded177905e5dc", @@ -761,12 +761,12 @@ "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudLinkRouting.kt": "d1a9fbf7d82b48ed227708221eec71c27dcc9828fb12467d132bde9a6a00f6f8", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudMediaViewer.kt": "f6b7bbbaf9269e1b22ae43aee40cd765e140f5f5fe2444894bcc79b2140bccad", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudMediaViewerActions.kt": "5c627c3980c086199aa583191d9623e3adc1f0dbbb9dc753cf6cbe176fe31667", - "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt": "4e0c16d3d7b5631c1e27eda274ab19feb1f3b33166c7a085d86feec5beda25b4", + "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNativeApp.kt": "700a83a6292b92ef48ce6a2bc9c7b88cbb01549ef891ff585361f4e1a8783339", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNotes.kt": "85bcc0e5bb26cd7a77982094b65972c212b7fb92e1e60bb690444f40b4f0f075", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudNotesCache.kt": "aa9ec330c3a569e2bc9948858ddde1d939358069d38fbd7a0319b1ecc3baa363", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPeople.kt": "cff910ea2cc77211ef81779c49ee0c957851f2b4a3ed32b857b12ded1cee643b", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPhotoEditor.kt": "70b83f289d3b5dbe4b6494b62827bb35b3f228066eeaf514092c0f9a6bfa9de1", - "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPlatform.kt": "f017fa7ab65c626963530805fd99fb7f3d2b43597b144910303fda201c558fc7", + "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudPlatform.kt": "96fb313f3380a2ebc9fbae38aaed562e61ddbd5646aca5355f325f48e794b935", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudResponseLimits.kt": "0d1605e5c2cfc43ef45d0ed4c20a7260d09b254efe1231bd21c65c36884ae69f", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudSession.kt": "792a381fd5eefc44e13eb73ee95a80f8d52e8dcec9d3876ba06ec5392b5c1f81", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/NextcloudSessionLoading.kt": "df00a758c8a29e9d1b221911a03f31547dc3510816da6228f8d38c607d0f4092", @@ -832,7 +832,7 @@ "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/TalkMessageTime.kt": "52a148d0d22dba02324bbfe429b927529086c469243f0cc42a4f63860685de6a", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/TalkMessages.kt": "26bc89d381d23c9f314aeb0f4c3ec9ebca2130ec7f31281b29b22dffa9d3ff35", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/TextEditorScreen.kt": "07dd6e7f285e5a4c06eed15b9cbeffe73eaf6121850c2f3d95577ba6ab68c169", - "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/TextEditorState.kt": "0ee67a706b8c072436fa107c4a1a2e8d762a48cace2d71cb11230e446d875c32", + "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/TextEditorState.kt": "a8218c153451d95b71dd16a83410aee8a921d9d004a16c063abbbef89caf4c83", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/TextFileDavSavePolicy.kt": "a47bd8440a7681a1ac2b20461789c2d36f886a7b4cd996a03acdaeaf2820a5c2", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/TrustedCertificateSettings.kt": "1ecc3c0397e2363a18a2e3e8f3721f6604a9909ab027323523e8a1601a630a97", "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/UnifiedSearch.kt": "0d9222d2765cc2a417c8563c00c44f9ddd104591d011ac5e30dad0aa4a546d81",