diff --git a/changes/unreleased/windows-cloud-files-blocked-commit.md b/changes/unreleased/windows-cloud-files-blocked-commit.md new file mode 100644 index 000000000..17963f51a --- /dev/null +++ b/changes/unreleased/windows-cloud-files-blocked-commit.md @@ -0,0 +1,7 @@ +category: fix +issue: none +pull: 517 +platforms: windows +user-facing: yes + +Windows Cloud Files now keeps writing back edits to a file that another application had open during an earlier upload, and marks that file as synced once the application closes it. diff --git a/changes/unreleased/windows-cloud-files-dehydration.md b/changes/unreleased/windows-cloud-files-dehydration.md new file mode 100644 index 000000000..741116d9f --- /dev/null +++ b/changes/unreleased/windows-cloud-files-dehydration.md @@ -0,0 +1,7 @@ +category: fix +issue: none +pull: 518 +platforms: windows +user-facing: yes + +Windows Cloud Files Free up space, automatic cleanup, and the storage summary now count and free downloaded files instead of finding nothing. Edited, pinned, and open files stay on the computer. diff --git a/docs/windows-release.md b/docs/windows-release.md index b1e0d7fa7..991d6aede 100644 --- a/docs/windows-release.md +++ b/docs/windows-release.md @@ -205,6 +205,18 @@ A file edited during the upload stays dirty and is uploaded again against the new revision, at most three times in one writeback before it reports a failed writeback. If the exclusive handle is unavailable, for example because another application has the file open, the commit is retried without uploading again. +If recording the new revision fails after the server accepted the upload, only +that update is retried, so the retry never uploads against the superseded +revision. Both retries are held in memory; after a restart, a placeholder whose +revision was not recorded is uploaded against its old revision and rejected by +the server. + +Until a blocked commit completes, the placeholder identity holds the uploaded +size, while the close-completion callback reports the size from the last +in-sync point. The close callback therefore does not require the two sizes to +match; hydration still does. On a local Windows 11 sync root, the close +notification that a blocking application sends when it releases the file +completes the pending commit. An application that restores an earlier last-write time while keeping the same size can defeat the stamp; this is an accepted limit of the guard. @@ -224,13 +236,44 @@ root. They cover: - an edit made while disconnected, written back at the next start; - a size-changing edit made while connected; -- an edit made during an upload. +- an edit made during an upload; +- a commit blocked by another process's open handle, which completes when that + handle closes, followed by a later edit. Each case must end with the placeholder in sync at the server's revision. `WindowsCloudFilesProviderTest` covers the remaining recovery and writeback behavior. These tests do not replace the Explorer and published-MSI acceptance checks above. +### Dehydration and storage accounting + +**Free up space**, automatic cleanup, and the Virtual files storage summary read +allocated bytes and dehydrate placeholders through +[`WindowsCloudDehydration`](../ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudDehydration.kt). +A connected file placeholder is a cloud reparse point, which Java NIO does not +report as a regular file, so both operations classify the path from native +find data with the same rule as the startup enumeration. Allocated bytes come +from `GetCompressedFileSizeW`: a dehydrated placeholder keeps its logical size +and allocates no bytes. + +Only an unpinned, in-sync file placeholder that allocates bytes is dehydrated, +so an online-only placeholder costs no native call. As +[`CfDehydratePlaceholder`](https://learn.microsoft.com/en-us/previous-versions/mt827480(v=vs.85)) +requires, dehydration holds an exclusive `CfOpenFileWithOplock` handle. When +the oplock cannot be taken (`0x80070020`) or breaks first, or when Windows +refuses a file that became dirty (`0x80070179`), pinned (`0x80070188`), or open +in another application (`0x80070187`), the content stays local and counts as +not freed. Automatic cleanup also keeps a planned file whose known revision, +pin, sync state, or allocated size changed after planning. Freed totals count +the bytes each dehydration actually released. + +**Last reviewed: 2026-10-17.** On a local Windows 11 sync root, +`WindowsCloudFilesDehydrationTest` hydrates a synthetic file, observes nonzero +allocated bytes, frees them through Free up space and through automatic cleanup, +and reopens the content. It also keeps a dirty edit, a pinned file, and a file +held open by another process local. This evidence does not cover Explorer's own +Free up space command or a published MSI. + ## WinGet delivery `tools/create-winget-manifests.ps1` generates a three-file WinGet manifest from diff --git a/tools/kotlin-file-size-baseline.txt b/tools/kotlin-file-size-baseline.txt index c94e42fc5..a624d759c 100644 --- a/tools/kotlin-file-size-baseline.txt +++ b/tools/kotlin-file-size-baseline.txt @@ -51,8 +51,8 @@ ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileSyncRemoteT ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt|6161 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|1030 -ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesProvider.kt|2338 +ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesJna.kt|1023 +ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesProvider.kt|2319 ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/DesktopVirtualRangeCacheTest.kt|2479 ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/JvmSupportIntakeTest.kt|2888 ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/LinuxVirtualFileSystemTest.kt|2149 diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudDehydration.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudDehydration.kt new file mode 100644 index 000000000..3435608d5 --- /dev/null +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudDehydration.kt @@ -0,0 +1,85 @@ +package dev.obiente.nextcloudnative.app + +import com.sun.jna.Native +import com.sun.jna.WString +import com.sun.jna.platform.win32.WinNT +import com.sun.jna.ptr.IntByReference +import java.nio.file.Path + +private const val FILE_ATTRIBUTE_PINNED = 0x0008_0000 + +/** + * Only an unpinned, in-sync file placeholder may release its local content. Dirty content is the + * only copy of a local edit and pinned content is kept on request, so neither is a candidate. + * + * The decision uses native find data: a connected file placeholder is a cloud reparse point that + * Java NIO reports as "other", never as a regular file. + */ +internal fun windowsCloudDehydratable(entry: WindowsCloudLocalEntry, fileAttributes: Int): Boolean = + entry.file && + fileAttributes and FILE_ATTRIBUTE_PINNED == 0 && + entry.inspection.state == WindowsCloudPlaceholderEntryState.InSync + +/** + * Windows keeps content that became dirty, pinned or opened by another application after + * [windowsCloudDehydratable] accepted it. The content stays local and nothing is freed. + */ +internal fun isWindowsCloudDehydrationRefusedResult(result: Int): Boolean = + result == 0x80070020.toInt() || // HRESULT_FROM_WIN32(ERROR_SHARING_VIOLATION): no exclusive oplock + result == 0x80070179.toInt() || // HRESULT_FROM_WIN32(ERROR_CLOUD_FILE_NOT_IN_SYNC) + result == 0x80070187.toInt() || // HRESULT_FROM_WIN32(ERROR_CLOUD_FILE_IN_USE) + result == 0x80070188.toInt() || // HRESULT_FROM_WIN32(ERROR_CLOUD_FILE_PINNED) + result == 0x80070323.toInt() // HRESULT_FROM_WIN32(ERROR_OPLOCK_HANDLE_CLOSED) + +/** + * CfDehydratePlaceholder requires an exclusive handle, and Microsoft recommends an exclusive + * oplock so another application's open breaks it instead of failing. Returns false when Windows + * keeps the content, including when the oplock cannot be taken or breaks before dehydration. + */ +internal fun CldApi.dehydrateExclusively(path: Path): Boolean { + val protectedHandle = WinNT.HANDLEByReference() + val flags = windowsCloudOpenFileFlags(write = false, exclusive = true) + val opened = CfOpenFileWithOplock(WString(path.toWindowsWin32Path()), flags, protectedHandle) + if (isWindowsCloudDehydrationRefusedResult(opened)) return false + checkWindowsCloudHResult(opened, "open a Windows Cloud Files placeholder for dehydration") + try { + // A reference fails once another application's open has broken the oplock. + if (CfReferenceProtectedHandle(protectedHandle.value).toInt() == 0) return false + try { + val handle = CfGetWin32HandleFromProtectedHandle(protectedHandle.value) + val result = CfDehydratePlaceholder(handle, 0L, -1L, 0, null) + if (isWindowsCloudDehydrationRefusedResult(result)) return false + checkWindowsCloudHResult(result, "dehydrate a Windows Cloud Files placeholder") + return true + } finally { + CfReleaseProtectedHandle(protectedHandle.value) + } + } finally { + CfCloseHandle(protectedHandle.value) + } +} + +/** + * Dehydrates the files an eviction plan selected, rechecking each at execution time. A file that + * another revision replaced, or that was pinned, edited or resized since planning, is kept. + * Returns the bytes actually released. + */ +internal fun WindowsCloudFilesApi.dehydratePlannedEvictions( + evictions: List, + currentFile: (relativePath: String) -> Pair?, +): Long = evictions.sumOf { eviction -> + val (identity, path) = currentFile(eviction.key.relativePath) ?: return@sumOf 0L + val unchanged = identity.remoteRevision == eviction.expectedLocalRevision && + !isPinned(path) && + placeholderState(path) == WindowsCloudPlaceholderState.InSync && + allocatedBytes(path) == eviction.sizeBytes + if (unchanged) dehydrate(path) else 0L +} + +/** Bytes allocated on disk; a dehydrated placeholder keeps its logical size but allocates none. */ +internal fun KernelFileApi.allocatedFileBytes(path: Path): Long { + val high = IntByReference() + val low = GetCompressedFileSizeW(WString(path.toWindowsWin32Path()), high) + if (low == -1 && Native.getLastError() != 0) return 0L + return ((high.value.toLong() and 0xffff_ffffL) shl 32) or (low.toLong() and 0xffff_ffffL) +} diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesJna.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesJna.kt index 59caa9459..ee25137e0 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesJna.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesJna.kt @@ -406,13 +406,8 @@ internal class JnaWindowsCloudFilesApi( override fun localChildren(directory: Path): List = localEntries(directory).map { it.path } - override fun allocatedBytes(path: Path): Long { - if (!Files.isRegularFile(path)) return 0L - val high = IntByReference() - val low = kernelFiles.GetCompressedFileSizeW(WString(path.toWindowsWin32Path()), high) - if (low == -1 && Native.getLastError() != 0) return 0L - return ((high.value.toLong() and 0xffff_ffffL) shl 32) or (low.toLong() and 0xffff_ffffL) - } + override fun allocatedBytes(path: Path): Long = + if (withFindData(path, false) { localEntry(path, it).file }) kernelFiles.allocatedFileBytes(path) else 0L override fun lastAccessedAtEpochMillis(path: Path): Long = withFindData(path, 0L) { findData -> findData.ftLastAccessTime.toTime().coerceAtLeast(0L) } @@ -528,17 +523,15 @@ internal class JnaWindowsCloudFilesApi( } override fun dehydrate(path: Path): Long { - if (!Files.isRegularFile(path)) return 0L - val size = Files.size(path) - withWindowsFileHandle(path, write = true) { handle -> - checkWindowsCloudHResult( - cldApi.CfDehydratePlaceholder(handle, 0L, -1L, 0, null), - "dehydrate a Windows Cloud Files placeholder", - ) - } - return size + if (!withFindData(path, false) { windowsCloudDehydratable(localEntry(path, it), it.dwFileAttributes) }) return 0L + // An online-only placeholder needs no native call, so a pass does not scale with the remote tree. + val allocated = kernelFiles.allocatedFileBytes(path).takeIf { it > 0L } ?: return 0L + return if (cldApi.dehydrateExclusively(path)) allocated else 0L } + private fun localEntry(path: Path, findData: WinBase.WIN32_FIND_DATA): WindowsCloudLocalEntry = + findData.windowsCloudLocalEntry(path, cldApi::CfGetPlaceholderStateFromFindData) + override fun close() { callbacksByConnection.keys.toList().forEach { key -> runCatching { disconnect(key) } } } diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesProvider.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesProvider.kt index ba1d65db6..09c00abd1 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesProvider.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesProvider.kt @@ -227,6 +227,7 @@ internal interface WindowsCloudFilesApi : AutoCloseable { fun localContentStamp(path: Path): WindowsCloudLocalContentStamp = nioWindowsCloudLocalContentStamp(path) fun markInSyncIfUnchanged(path: Path, expected: WindowsCloudLocalContentStamp): WindowsCloudInSyncCommit = markInSyncWhenStampMatches(path, expected) + /** Returns the bytes released, or 0 when the entry is not an unpinned in-sync file or Windows kept it. */ fun dehydrate(path: Path): Long } @@ -906,7 +907,8 @@ internal class WindowsCloudFilesProvider( override fun closed(info: WindowsCloudCallbackInfo, deleted: Boolean) { if (callbacksPaused.get() || deleted || info.fileIdentity == null) return - val identity = runCatching { requireIdentity(info, expectDirectory = null) }.getOrNull() ?: return + // After a blocked commit Windows still reports the last in-sync size; only hydration needs it. + val identity = runCatching { requireIdentity(info, null, recordedSize = false) }.getOrNull() ?: return if (identity.directory) return val localPath = root.resolve(identity.path.replace('/', File.separatorChar)).normalize() if (!localPath.startsWith(root) || !Files.exists(localPath)) return @@ -915,12 +917,7 @@ internal class WindowsCloudFilesProvider( if (api.placeholderState(localPath) != WindowsCloudPlaceholderState.Dirty) return pendingWritebacks += identity.path submitPathOperation(identity.path) { - val current = api.placeholderIdentity(localPath) - ?.let { encoded -> runCatching { WindowsCloudFileIdentityCodec.decode(encoded) }.getOrNull() } - ?.takeIf { it.accountId == backend.accountId && it.path == identity.path && !it.directory } - ?: knownIdentities[identity.path] - ?: identity - writeback.writeBack(localPath, current) + writeback.writeBack(localPath, identity.path, fallback = knownIdentities[identity.path] ?: identity) } } } @@ -1041,9 +1038,7 @@ internal class WindowsCloudFilesProvider( if (requestedBytes > 0L && freed >= requestedBytes) return@forEach val path = localPath(identity) if (path.startsWith(root) && api.placeholderState(path) == WindowsCloudPlaceholderState.InSync) { - val allocated = api.allocatedBytes(path) - api.dehydrate(path) - freed += allocated + freed += api.dehydrate(path) } } return freed @@ -1082,20 +1077,9 @@ internal class WindowsCloudFilesProvider( availableFreeBytes = Files.getFileStore(root).usableSpace, nowEpochMillis = nowEpochMillis, ) - var freed = 0L - plan.evictions.forEach { eviction -> - val identity = knownIdentities[eviction.key.relativePath] ?: return@forEach - val path = localPath(identity) - if ( - !api.isPinned(path) && - api.placeholderState(path) == WindowsCloudPlaceholderState.InSync && - api.allocatedBytes(path) == eviction.sizeBytes - ) { - api.dehydrate(path) - freed += eviction.sizeBytes - } + return api.dehydratePlannedEvictions(plan.evictions) { relative -> + knownIdentities[relative]?.let { identity -> identity to localPath(identity) } } - return freed } fun summary(): WindowsCloudFilesSummary = synchronized(corruptRootStableAccessLock) { @@ -1847,13 +1831,14 @@ internal class WindowsCloudFilesProvider( private fun requireIdentity( info: WindowsCloudCallbackInfo, expectDirectory: Boolean?, + recordedSize: Boolean = true, ): WindowsCloudFileIdentity { val bytes = requireNotNull(info.fileIdentity) { "The Cloud Files callback has no identity." } val identity = WindowsCloudFileIdentityCodec.decode(bytes) require(identity.accountId == backend.accountId) { "The Cloud Files callback belongs to another account." } requireWindowsCloudCallbackPath(root, info.normalizedPath, identity.path) if (expectDirectory != null) require(identity.directory == expectDirectory) - require(identity.size == info.fileSize || identity.directory) { "The Cloud Files callback size is stale." } + require(!recordedSize || identity.size == info.fileSize || identity.directory) { "The Cloud Files callback size is stale." } knownIdentities[identity.path] = identity return identity } @@ -2106,11 +2091,7 @@ internal class WindowsCloudFilesProvider( deferredWhenPaused = { if (!runtimeStopping.get()) deferredLocalChanges.add(local) }, allowWhilePaused = allowWhilePaused, ) { - val current = requireNotNull(api.placeholderIdentity(local)) { - "The dirty Windows placeholder has no recoverable identity." - }.let(WindowsCloudFileIdentityCodec::decode) - require(current.path == original.path) { "The dirty Windows placeholder identity is not safe to recover." } - writeback.writeBack(local, current) + writeback.writeBack(local, original.path) } } } diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesWriteback.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesWriteback.kt index e5bc8d023..8e326658c 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesWriteback.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesWriteback.kt @@ -52,7 +52,8 @@ internal fun WindowsCloudFilesApi.markInSyncWhenStampMatches( * accepts the upload, the placeholder receives the new revision while it stays dirty, which is * correct whether or not the local file changed. It is marked in sync only while an exclusive * handle proves the stamp is unchanged. A file edited meanwhile stays dirty and uploads again - * against the new revision. A commit that cannot get its handle is retried without uploading again. + * against the new revision. A placeholder update or commit that fails after the server accepted the + * upload is retried without uploading again, so the retry never reuses the superseded revision. */ internal class WindowsCloudFilesWriteback( private val api: WindowsCloudFilesApi, @@ -61,12 +62,26 @@ internal class WindowsCloudFilesWriteback( private val recordUploadedRevision: (Path, WindowsCloudFileIdentity) -> Boolean, ) { private data class PendingCommit( + val base: WindowsCloudFileIdentity, val uploaded: WindowsCloudFileIdentity, val stamp: WindowsCloudLocalContentStamp, ) private val pendingCommits = ConcurrentHashMap() + /** + * Writes back the dirty placeholder for [path] using the identity it currently holds, which a + * previous writeback may have advanced. Only a missing or unreadable identity uses [fallback]. + */ + fun writeBack(local: Path, path: String, fallback: WindowsCloudFileIdentity? = null) { + val current = api.placeholderIdentity(local) + ?.let { encoded -> runCatching { WindowsCloudFileIdentityCodec.decode(encoded) }.getOrNull() } + ?.takeIf { it.accountId == backend.accountId && it.path == path && !it.directory } + ?: fallback + ?: error("The dirty Windows placeholder has no recoverable identity.") + writeBack(local, current) + } + /** Runs inside the path operation for [current]'s path; [current] is the placeholder identity. */ fun writeBack(local: Path, current: WindowsCloudFileIdentity) { require(current.accountId == backend.accountId && !current.directory) { @@ -76,15 +91,21 @@ internal class WindowsCloudFilesWriteback( repeat(MAX_UPLOADS_PER_WRITEBACK) { // A queued writeback may find an earlier one already committed the same content. if (api.placeholderState(local) != WindowsCloudPlaceholderState.Dirty) return - val pending = pendingCommits[target.path]?.takeIf { it.uploaded == target } ?: upload(local, target) ?: return - when (api.markInSyncIfUnchanged(local, pending.stamp)) { + val pending = pendingCommits[target.path] + val accepted = when (target) { + pending?.uploaded -> pending + // The server accepted this upload, but recording its revision failed: retry only that. + pending?.base -> pending?.takeIf { recordUploadedRevision(local, it.uploaded) } + else -> upload(local, target) + } ?: return handOverToRootRecovery(target.path) + when (api.markInSyncIfUnchanged(local, accepted.stamp)) { WindowsCloudInSyncCommit.Committed -> { - pendingCommits.remove(target.path, pending) + pendingCommits.remove(target.path, accepted) return } WindowsCloudInSyncCommit.LocalChanged -> { - pendingCommits.remove(target.path, pending) - target = pending.uploaded + pendingCommits.remove(target.path, accepted) + target = accepted.uploaded } } } @@ -93,14 +114,20 @@ internal class WindowsCloudFilesWriteback( fun clear() = pendingCommits.clear() + /** A corrupt placeholder update scheduled root recovery, which rebuilds from the server. */ + private fun handOverToRootRecovery(path: String) { + pendingCommits.remove(path) + } + /** Returns null when a corrupt placeholder update handed the commit to root recovery. */ private fun upload(local: Path, current: WindowsCloudFileIdentity): PendingCommit? { pendingCommits.remove(current.path) val stamp = api.localContentStamp(local) val uploaded = backend.upload(current.path, local.toFile(), current.remoteRevision) recordIdentity(uploaded) - if (!recordUploadedRevision(local, uploaded)) return null - return PendingCommit(uploaded, stamp).also { pendingCommits[current.path] = it } + // Recorded before the placeholder update, which can fail after the server accepted the upload. + val pending = PendingCommit(current, uploaded, stamp).also { pendingCommits[current.path] = it } + return pending.takeIf { recordUploadedRevision(local, uploaded) } } private companion object { diff --git a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudLocalTraversal.kt b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudLocalTraversal.kt index 817dafb36..be90c2ba3 100644 --- a/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudLocalTraversal.kt +++ b/ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudLocalTraversal.kt @@ -109,6 +109,13 @@ internal fun WinBase.WIN32_FIND_DATA.windowsCloudInspection( ) } +/** Classifies one native find-data record; see [windowsCloudLocalEntry]. */ +internal fun WinBase.WIN32_FIND_DATA.windowsCloudLocalEntry( + path: Path, + placeholderState: (Pointer) -> Int, +): WindowsCloudLocalEntry = + windowsCloudLocalEntry(path, dwFileAttributes, dwReserved0, windowsCloudInspection(placeholderState)) + /** * Enumerates resident children with their find data in one native call, without asking Cloud * Files to populate remote folders. @@ -132,10 +139,7 @@ internal fun windowsCloudLocalEntries( do { data.read() val name = Native.toString(data.cFileName) - if (name != "." && name != "..") { - val inspection = data.windowsCloudInspection(placeholderState) - add(windowsCloudLocalEntry(directory.resolve(name), data.dwFileAttributes, data.dwReserved0, inspection)) - } + if (name != "." && name != "..") add(data.windowsCloudLocalEntry(directory.resolve(name), placeholderState)) } while (Kernel32.INSTANCE.FindNextFile(handle, data.pointer)) val error = Native.getLastError() if (error != 18) throw WindowsCloudLocalScanException(error) diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudExternalProcess.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudExternalProcess.kt new file mode 100644 index 000000000..c482f4955 --- /dev/null +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudExternalProcess.kt @@ -0,0 +1,52 @@ +package dev.obiente.nextcloudnative.app + +import java.nio.file.Path +import java.util.Base64 +import java.util.concurrent.TimeUnit + +/** + * Native Cloud Files tests act on a sync root from another process, as Explorer or an editor + * would. Cloud Files sends no close notification for I/O by the connected provider's own process. + */ +internal fun runExternalPowerShell(script: String, environment: Map) { + val process = ProcessBuilder("powershell.exe", "-NoProfile", "-NonInteractive", "-Command", script) + .redirectErrorStream(true) + .apply { environment().putAll(environment) } + .start() + val output = process.inputStream.bufferedReader().readText() + check(process.waitFor(30, TimeUnit.SECONDS) && process.exitValue() == 0) { "External process failed: $output" } +} + +/** Overwrites from offset zero without truncating, like an editor saving a longer file. */ +internal fun editInPlaceFromAnotherProcess(path: Path, bytes: ByteArray) = runExternalPowerShell( + "\$s = [IO.File]::Open(\$env:NATIVE_EDIT_PATH, 'Open', 'Write', 'ReadWrite');" + + "\$b = [Convert]::FromBase64String(\$env:NATIVE_EDIT_BYTES); \$s.Write(\$b, 0, \$b.Length); \$s.Close()", + mapOf( + "NATIVE_EDIT_PATH" to path.toAbsolutePath().toString(), + "NATIVE_EDIT_BYTES" to Base64.getEncoder().encodeToString(bytes), + ), +) + +/** + * Holds a read handle in another process until closed, as an open document would. [share] is the + * .NET FileShare mode granted to other openers. + */ +internal class ExternalOpenHandle(path: Path, share: String = "ReadWrite") : AutoCloseable { + private val process = ProcessBuilder( + "powershell.exe", "-NoProfile", "-NonInteractive", "-Command", + "\$s = [IO.File]::Open(\$env:NATIVE_HOLD_PATH, 'Open', 'Read', '$share'); " + + "[Console]::Out.WriteLine('open'); [Console]::Out.Flush(); [void][Console]::In.ReadLine(); \$s.Close()", + ).redirectErrorStream(true) + .apply { environment()["NATIVE_HOLD_PATH"] = path.toAbsolutePath().toString() } + .start() + + init { + val ready = process.inputStream.bufferedReader().readLine() + check(ready == "open") { "The external handle did not open: $ready" } + } + + override fun close() { + process.outputStream.bufferedWriter().use { it.write("close"); it.newLine() } + check(process.waitFor(30, TimeUnit.SECONDS) && process.exitValue() == 0) { "The external handle did not close." } + } +} diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesDehydrationTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesDehydrationTest.kt new file mode 100644 index 000000000..c38818ff7 --- /dev/null +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesDehydrationTest.kt @@ -0,0 +1,250 @@ +package dev.obiente.nextcloudnative.app + +import com.sun.jna.Native +import com.sun.jna.WString +import com.sun.jna.ptr.IntByReference +import java.nio.file.Files +import java.nio.file.Path +import java.util.concurrent.TimeUnit +import kotlin.io.path.createTempDirectory +import kotlin.test.Test +import kotlin.test.assertContentEquals +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue +import org.junit.Assume.assumeTrue + +/** + * Free up space, automatic cleanup and the storage summary on connected placeholders. While the + * provider is connected, every file placeholder is a cloud reparse point that Java NIO reports as + * "other". Native cases run only on Windows, against a disposable sync root and a synthetic server. + */ +class WindowsCloudFilesDehydrationTest { + private fun dehydratable(attributes: Int, tag: Int, state: WindowsCloudPlaceholderEntryState): Boolean { + val inspection = WindowsCloudPlaceholderInspection(state) + return windowsCloudDehydratable(windowsCloudLocalEntry(Path.of("synthetic-entry"), attributes, tag, inspection), attributes) + } + + @Test + fun `only an unpinned in-sync file placeholder is dehydratable`() { + // Attributes and tags as measured on a connected Windows 11 sync root. + assertTrue(dehydratable(HYDRATED, HYDRATED_TAG, WindowsCloudPlaceholderEntryState.InSync)) + assertTrue(dehydratable(DEHYDRATED, DEHYDRATED_TAG, WindowsCloudPlaceholderEntryState.InSync)) + assertFalse(dehydratable(HYDRATED, DIRTY_TAG, WindowsCloudPlaceholderEntryState.Dirty)) + assertFalse(dehydratable(HYDRATED or FILE_ATTRIBUTE_PINNED, HYDRATED_TAG, WindowsCloudPlaceholderEntryState.InSync)) + // A plain file is not a placeholder, and links, folders and foreign reparse points are not files. + assertFalse(dehydratable(FILE_ATTRIBUTE_ARCHIVE, 0, WindowsCloudPlaceholderEntryState.Local)) + assertFalse(dehydratable(FILE_ATTRIBUTE_REPARSE_POINT, IO_REPARSE_TAG_SYMLINK, WindowsCloudPlaceholderEntryState.InSync)) + assertFalse(dehydratable(FILE_ATTRIBUTE_DIRECTORY or FILE_ATTRIBUTE_REPARSE_POINT, HYDRATED_TAG, WindowsCloudPlaceholderEntryState.InSync)) + assertFalse(dehydratable(HYDRATED, IO_REPARSE_TAG_DEDUP, WindowsCloudPlaceholderEntryState.InSync)) + } + + @Test + fun `Windows refusals of dirty, pinned and open content free nothing`() { + assertTrue(isWindowsCloudDehydrationRefusedResult(0x80070179.toInt())) + assertTrue(isWindowsCloudDehydrationRefusedResult(0x80070187.toInt())) + assertTrue(isWindowsCloudDehydrationRefusedResult(0x80070188.toInt())) + // The exclusive oplock is unavailable or broke before dehydration. + assertTrue(isWindowsCloudDehydrationRefusedResult(0x80070020.toInt())) + assertTrue(isWindowsCloudDehydrationRefusedResult(0x80070323.toInt())) + assertFalse(isWindowsCloudDehydrationRefusedResult(0)) + assertFalse(isWindowsCloudDehydrationRefusedResult(0x80070005.toInt())) // E_ACCESSDENIED + assertFalse(isWindowsCloudDehydrationRefusedResult(0x8007016B.toInt())) // Metadata corrupt + } + + @Test + fun `a planned eviction is kept when its file changed after planning`() { + val planned = WindowsCloudFileIdentity("account-01", "planned.bin", "\"etag-01\"", 64L, false) + val api = EvictionApi(allocated = 64L) + fun execute(current: WindowsCloudFileIdentity?) = api.dehydratePlannedEvictions(listOf(eviction(planned))) { relative -> + current?.takeIf { it.path == relative }?.let { it to Path.of(it.path) } + } + + // A same-size newer revision replaced the planned one, so its access history is unknown. + assertEquals(0L, execute(planned.copy(remoteRevision = "\"etag-02\""))) + assertEquals(0L, execute(null)) + api.pinned = true + assertEquals(0L, execute(planned)) + api.pinned = false + api.state = WindowsCloudPlaceholderState.Dirty + assertEquals(0L, execute(planned)) + api.state = WindowsCloudPlaceholderState.InSync + api.allocated = 128L + assertEquals(0L, execute(planned)) + assertEquals(emptyList(), api.dehydrated) + + api.allocated = 64L + assertEquals(64L, execute(planned)) + assertEquals(listOf(Path.of("planned.bin")), api.dehydrated) + // Freed bytes are what dehydration released, not what the plan expected. + api.released = 0L + assertEquals(0L, execute(planned)) + } + + @Test + fun `native Windows frees up space from a hydrated placeholder that still opens`() = + withNativeRoot { root, _, api, provider -> + val photo = root.resolve("photo.bin") + assertContentEquals(CONTENT, Files.readAllBytes(photo)) + val allocated = awaitAllocated(api, photo) + provider.summary().let { summary -> + assertEquals(1, summary.hydratedFileCount) + assertEquals(allocated, summary.cachedBytes) + assertEquals(allocated, summary.reclaimableBytes) + } + + assertEquals(allocated, provider.freeUpSpace(requestedBytes = 0L)) + + assertEquals(0L, api.allocatedBytes(photo)) + assertEquals(WindowsCloudPlaceholderState.InSync, api.placeholderState(photo)) + assertEquals(0L, provider.summary().cachedBytes) + assertContentEquals(CONTENT, Files.readAllBytes(photo)) + assertEquals(allocated, awaitAllocated(api, photo)) + } + + @Test + fun `native Windows automatic cleanup dehydrates content over its budget`() = + withNativeRoot { root, _, api, provider -> + val photo = root.resolve("photo.bin") + assertContentEquals(CONTENT, Files.readAllBytes(photo)) + val allocated = awaitAllocated(api, photo) + val policy = VirtualFileCachePolicy(maximumCacheBytes = 1L, minimumFreeSpaceBytes = 0L, unusedFileAgeMillis = null) + + assertEquals(allocated, provider.enforcePolicy(policy)) + + assertEquals(0L, api.allocatedBytes(photo)) + assertContentEquals(CONTENT, Files.readAllBytes(photo)) + } + + @Test + fun `native Windows keeps dirty, pinned and open content local`() = + withNativeRoot { root, backend, api, provider -> + val draft = root.resolve("draft.bin") + val pinned = root.resolve("pinned.bin") + val open = root.resolve("open.bin") + listOf(draft, pinned, open).forEach { assertContentEquals(CONTENT, Files.readAllBytes(it)) } + runExternalPowerShell("attrib +P \$env:NATIVE_PIN_PATH", mapOf("NATIVE_PIN_PATH" to pinned.toAbsolutePath().toString())) + assertTrue(api.isPinned(pinned)) + // The server stays unavailable, so the edit remains the only copy. + backend.unavailableUploads = Int.MAX_VALUE + editInPlaceFromAnotherProcess(draft, EDIT) + awaitCondition({ "dirty draft" }) { api.placeholderState(draft) == WindowsCloudPlaceholderState.Dirty } + val allocated = listOf(draft, pinned, open).associateWith { awaitAllocated(api, it) } + + // An application that denies write sharing keeps the file open; Windows reports it in use. + ExternalOpenHandle(open, share = "Read").use { + assertEquals(0L, api.dehydrate(open)) + assertEquals(0L, provider.freeUpSpace(requestedBytes = 0L)) + assertEquals(0L, provider.enforcePolicy(VirtualFileCachePolicy(maximumCacheBytes = 1L, minimumFreeSpaceBytes = 0L))) + } + assertEquals(0L, api.dehydrate(draft)) + assertEquals(0L, api.dehydrate(pinned)) + + assertEquals(allocated, listOf(draft, pinned, open).associateWith(api::allocatedBytes)) + assertEquals(WindowsCloudPlaceholderState.Dirty, api.placeholderState(draft)) + assertContentEquals(EDIT, Files.readAllBytes(draft).copyOf(EDIT.size)) + provider.summary().let { summary -> + assertEquals(allocated.values.sum(), summary.cachedBytes) + assertEquals(allocated.getValue(open), summary.reclaimableBytes) + assertEquals(allocated.getValue(pinned), summary.pinnedBytes) + } + } + + private fun eviction(identity: WindowsCloudFileIdentity) = VirtualFileEviction( + key = FileOfflineKey(identity.accountId, identity.path), + expectedLocalRevision = identity.remoteRevision, + sizeBytes = identity.size, + reasons = setOf(VirtualFileEvictionReason.CacheBudget), + ) + + /** Only the checks an eviction executor makes; every other Cloud Files call is unexpected. */ + private class EvictionApi( + var allocated: Long, + ) : WindowsCloudFilesApi by WindowsCloudFilesProviderTest.FakeApi() { + var pinned = false + var state = WindowsCloudPlaceholderState.InSync + var released = allocated + val dehydrated = mutableListOf() + + override fun isPinned(path: Path) = pinned + override fun placeholderState(path: Path) = state + override fun allocatedBytes(path: Path) = allocated + override fun dehydrate(path: Path): Long = released.also { dehydrated.add(path) } + } + + private fun withNativeRoot( + block: ( + Path, + WindowsCloudFilesPlaceholderRecoveryTest.RevisionedBackend, + JnaWindowsCloudFilesApi, + WindowsCloudFilesProvider, + ) -> Unit, + ) { + assumeTrue(isWindowsDesktop()) + val root = createTempDirectory("cloud-native-dehydration-") + val backend = WindowsCloudFilesPlaceholderRecoveryTest.RevisionedBackend( + *listOf("photo.bin", "draft.bin", "pinned.bin", "open.bin").map { it to CONTENT }.toTypedArray(), + ) + val api = JnaWindowsCloudFilesApi(shellRegistrar = PackagedWindowsCloudShellRegistrar(launcherPath = null)) + // Failed uploads are not retried during a test, so a dirty edit stays dirty. + val provider = WindowsCloudFilesProvider(root, backend, api, writebackRetryDelayMillis = { TimeUnit.HOURS.toMillis(1) }) + try { + provider.start() + provider.recoverAfterStartup(timeoutSeconds = 5) + block(root, backend, api, provider) + } finally { + try { + provider.removeSyncRoot() + } finally { + provider.close() + root.toFile().deleteRecursively() + } + } + } + + /** + * Waits, within a bound, until a hydrated file reports its whole size as allocated. Reading the + * content does not guarantee that a single allocation sample already includes every cluster. + */ + private fun awaitAllocated(api: JnaWindowsCloudFilesApi, path: Path): Long { + awaitCondition({ "allocated bytes of ${path.fileName}: ${allocationDiagnostics(api, path)}" }) { + api.allocatedBytes(path) >= CONTENT.size + } + return api.allocatedBytes(path) + } + + private fun allocationDiagnostics(api: JnaWindowsCloudFilesApi, path: Path): String { + val high = IntByReference() + val low = Native.load("kernel32", KernelFileApi::class.java) + .GetCompressedFileSizeW(WString(path.toWindowsWin32Path()), high) + val error = Native.getLastError() + return "reported=${api.allocatedBytes(path)}, raw=$low/${high.value}, error=$error, " + + "pinned=${api.isPinned(path)}, inspection=${api.inspectPlaceholder(path)}" + } + + private fun awaitCondition(label: () -> String, condition: () -> Boolean) { + val deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(15) + while (!condition()) { + check(System.nanoTime() < deadline) { "Timed out waiting for ${label()}." } + Thread.sleep(10) + } + } + + private companion object { + const val FILE_ATTRIBUTE_DIRECTORY = 0x10 + const val FILE_ATTRIBUTE_ARCHIVE = 0x20 + const val FILE_ATTRIBUTE_REPARSE_POINT = 0x400 + const val FILE_ATTRIBUTE_PINNED = 0x0008_0000 + const val HYDRATED = FILE_ATTRIBUTE_ARCHIVE or FILE_ATTRIBUTE_REPARSE_POINT + const val DEHYDRATED = 0x0040_1620 // Adds sparse, offline and recall-on-data-access. + const val HYDRATED_TAG = 0x9000601A.toInt() + const val DEHYDRATED_TAG = 0x9000401A.toInt() + const val DIRTY_TAG = 0x9000201A.toInt() + const val IO_REPARSE_TAG_SYMLINK = 0xA000000C.toInt() + const val IO_REPARSE_TAG_DEDUP = 0x80000013.toInt() + + // Large enough that a hydrated placeholder allocates whole clusters. + val CONTENT = ByteArray(256 * 1024) { (it % 251).toByte() } + val EDIT = "synthetic local edit".encodeToByteArray() + } +} diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesRuntimeWritebackTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesRuntimeWritebackTest.kt index 439a9ee5a..99d777664 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesRuntimeWritebackTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesRuntimeWritebackTest.kt @@ -2,25 +2,49 @@ package dev.obiente.nextcloudnative.app import java.nio.file.Files import java.nio.file.Path -import java.util.Base64 import java.util.concurrent.TimeUnit import kotlin.io.path.createTempDirectory +import kotlin.io.path.writeBytes import kotlin.test.Test import kotlin.test.assertContentEquals import kotlin.test.assertEquals +import kotlin.test.assertTrue import org.junit.Assume.assumeTrue /** - * Native writeback of placeholder edits made by another process while the provider is connected. - * These run only on Windows, against a disposable sync root and a synthetic server. + * Writeback of placeholder edits made while the provider is connected. Native cases run only on + * Windows, against a disposable sync root and a synthetic server, with edits from another process. */ class WindowsCloudFilesRuntimeWritebackTest { @Test - fun `native Windows writes back an edit that changes the file size`() = withNativeRoot { root, backend, api -> + fun `a close callback reporting the last in-sync size still writes back the edit`() { + val root = createTempDirectory("windows-cloud-runtime-size-") + try { + val backend = WindowsCloudFilesPlaceholderRecoveryTest.RevisionedBackend("edit.txt" to ORIGINAL_BYTES) + val local = root.resolve("edit.txt").apply { writeBytes(NEWER_EDIT) } + // After a blocked commit the identity carries the uploaded size, not the last in-sync size. + val recorded = backend.identity("edit.txt").copy(size = LONGER_EDIT.size.toLong()) + val api = WindowsCloudFilesProviderTest.FakeApi(expectedConversions = 1).apply { + seed(local, WindowsCloudPlaceholderState.Dirty, recorded) + } + WindowsCloudFilesProvider(root, backend, api).use { provider -> + provider.closed(closeInfo(local, recorded, fileSize = ORIGINAL_BYTES.size.toLong()), deleted = false) + + assertTrue(api.awaitConversions()) + assertEquals(listOf(ORIGINAL_REVISION), backend.uploadRevisions) + assertContentEquals(NEWER_EDIT, backend.content("edit.txt")) + } + } finally { + root.toFile().deleteRecursively() + } + } + + @Test + fun `native Windows writes back an edit that changes the file size`() = withNativeRoot { root, backend, api, _ -> val draft = root.resolve("draft.txt") assertContentEquals(ORIGINAL_BYTES, Files.readAllBytes(draft)) - editInPlace(draft, LONGER_EDIT) + editInPlaceFromAnotherProcess(draft, LONGER_EDIT) awaitInSync(api, draft, backend, commits = 1) assertEquals(listOf(ORIGINAL_REVISION), backend.uploadRevisions) @@ -30,13 +54,13 @@ class WindowsCloudFilesRuntimeWritebackTest { @Test fun `native Windows uploads an edit made during a writeback before marking the file in sync`() = - withNativeRoot { root, backend, api -> + withNativeRoot { root, backend, api, _ -> val draft = root.resolve("draft.txt") assertContentEquals(ORIGINAL_BYTES, Files.readAllBytes(draft)) var edits = 1 - backend.duringUpload = { if (edits-- > 0) editInPlace(draft, NEWER_EDIT) } + backend.duringUpload = { if (edits-- > 0) editInPlaceFromAnotherProcess(draft, NEWER_EDIT) } - editInPlace(draft, LONGER_EDIT) + editInPlaceFromAnotherProcess(draft, LONGER_EDIT) awaitInSync(api, draft, backend, commits = 2) assertEquals(listOf(ORIGINAL_REVISION, "\"server-1\""), backend.uploadRevisions.distinct()) @@ -44,8 +68,41 @@ class WindowsCloudFilesRuntimeWritebackTest { assertContentEquals(NEWER_EDIT, Files.readAllBytes(draft)) } + @Test + fun `native Windows completes a commit blocked by another application once it closes the file`() = + withNativeRoot { root, backend, api, provider -> + val draft = root.resolve("draft.txt") + assertContentEquals(ORIGINAL_BYTES, Files.readAllBytes(draft)) + + // Another application keeps the file open, so the exclusive in-sync commit cannot run. + ExternalOpenHandle(draft).use { + editInPlaceFromAnotherProcess(draft, LONGER_EDIT) + awaitCondition("blocked commit") { + backend.commits == 1 && provider.summary().failedWritebackCount == 1 + } + assertEquals(WindowsCloudPlaceholderState.Dirty, api.placeholderState(draft)) + } + + // Its close reports the size from the last in-sync point, not the uploaded size, and + // completes the pending commit without uploading again. + awaitInSync(api, draft, backend, commits = 1) + assertEquals(listOf(ORIGINAL_REVISION), backend.uploadRevisions) + assertContentEquals(LONGER_EDIT, backend.content("draft.txt")) + + editInPlaceFromAnotherProcess(draft, NEWER_EDIT) + + awaitInSync(api, draft, backend, commits = 2) + assertEquals(listOf(ORIGINAL_REVISION, "\"server-1\""), backend.uploadRevisions) + assertContentEquals(NEWER_EDIT, backend.content("draft.txt")) + } + private fun withNativeRoot( - block: (Path, WindowsCloudFilesPlaceholderRecoveryTest.RevisionedBackend, JnaWindowsCloudFilesApi) -> Unit, + block: ( + Path, + WindowsCloudFilesPlaceholderRecoveryTest.RevisionedBackend, + JnaWindowsCloudFilesApi, + WindowsCloudFilesProvider, + ) -> Unit, ) { assumeTrue(isWindowsDesktop()) val root = createTempDirectory("cloud-native-runtime-writeback-") @@ -55,7 +112,7 @@ class WindowsCloudFilesRuntimeWritebackTest { try { provider.start() provider.recoverAfterStartup(timeoutSeconds = 5) - block(root, backend, api) + block(root, backend, api, provider) assertEquals(0, provider.summary().failedWritebackCount) } finally { try { @@ -67,6 +124,14 @@ class WindowsCloudFilesRuntimeWritebackTest { } } + private fun awaitCondition(label: String, condition: () -> Boolean) { + val deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(15) + while (!condition()) { + check(System.nanoTime() < deadline) { "Timed out waiting for $label." } + Thread.sleep(10) + } + } + private fun awaitInSync( api: WindowsCloudFilesApi, path: Path, @@ -83,24 +148,15 @@ class WindowsCloudFilesRuntimeWritebackTest { } } - /** - * Overwrites from offset zero without truncating, like an editor saving a longer file. Cloud - * Files sends no close notification for I/O by the connected provider's own process, so the - * edit runs in a separate process as a real editor would. - */ - private fun editInPlace(path: Path, bytes: ByteArray) { - val script = "\$s = [IO.File]::Open(\$env:NATIVE_EDIT_PATH, 'Open', 'Write'); " + - "\$b = [Convert]::FromBase64String(\$env:NATIVE_EDIT_BYTES); \$s.Write(\$b, 0, \$b.Length); \$s.Close()" - val process = ProcessBuilder("powershell.exe", "-NoProfile", "-NonInteractive", "-Command", script) - .redirectErrorStream(true) - .apply { - environment()["NATIVE_EDIT_PATH"] = path.toAbsolutePath().toString() - environment()["NATIVE_EDIT_BYTES"] = Base64.getEncoder().encodeToString(bytes) - } - .start() - val output = process.inputStream.bufferedReader().readText() - check(process.waitFor(30, TimeUnit.SECONDS) && process.exitValue() == 0) { "External edit failed: $output" } - } + private fun closeInfo(local: Path, identity: WindowsCloudFileIdentity, fileSize: Long) = WindowsCloudCallbackInfo( + connectionKey = 1L, + transferKey = 2L, + requestKey = 3L, + normalizedPath = local.toString(), + fileIdentity = WindowsCloudFileIdentityCodec.encode(identity), + fileSize = fileSize, + priorityHint = 0, + ) private companion object { const val ORIGINAL_REVISION = "\"etag-01\"" diff --git a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesWritebackTest.kt b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesWritebackTest.kt index 5dcef5004..d92285470 100644 --- a/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesWritebackTest.kt +++ b/ui/src/desktopTest/kotlin/dev/obiente/nextcloudnative/app/WindowsCloudFilesWritebackTest.kt @@ -75,6 +75,21 @@ class WindowsCloudFilesWritebackTest { assertEquals(listOf("revision:\"server-1\"", "in-sync"), fixture.events) } + @Test + fun `a failed revision update after an accepted upload is retried without uploading again`() = withFile { local -> + val fixture = Fixture(local) + fixture.revisionFailures = 1 + + assertFailsWith { fixture.writeback.writeBack(local, fixture.original) } + // The placeholder still holds the superseded revision; uploading against it would be rejected. + assertEquals(fixture.original, fixture.placeholderIdentity()) + + fixture.writeback.writeBack(local, fixture.placeholderIdentity()) + + assertEquals(listOf(ORIGINAL), fixture.backend.revisions) + assertEquals(listOf("revision:\"server-1\"", "in-sync"), fixture.events) + } + @Test fun `a pending commit is dropped once the placeholder holds another revision`() = withFile { local -> val fixture = Fixture(local) @@ -110,6 +125,24 @@ class WindowsCloudFilesWritebackTest { assertEquals(listOf("revision:\"server-1\""), fixture.events) } + @Test + fun `a path writeback uses the identity the placeholder holds`() = withFile { local -> + val fixture = Fixture(local) + + fixture.writeback.writeBack(local, "edit.txt", fallback = fixture.original.copy(remoteRevision = "\"stale\"")) + + assertEquals(listOf(ORIGINAL), fixture.backend.revisions) + } + + @Test + fun `a path writeback without a matching identity or fallback uploads nothing`() = withFile { local -> + val fixture = Fixture(local) + + assertFailsWith { fixture.writeback.writeBack(local, "other.txt") } + + assertEquals(emptyList(), fixture.backend.revisions) + } + @Test fun `a directory identity is never written back`() = withFile { local -> val fixture = Fixture(local) @@ -133,6 +166,7 @@ class WindowsCloudFilesWritebackTest { val backend = RevisionedServer() val events = mutableListOf() var commitFailures = 0 + var revisionFailures = 0 private var placeholder = original private val fake = WindowsCloudFilesProviderTest.FakeApi().apply { seed(local, WindowsCloudPlaceholderState.Dirty, original) @@ -152,6 +186,10 @@ class WindowsCloudFilesWritebackTest { } val writeback = WindowsCloudFilesWriteback(api, backend, {}) { path, uploaded -> check(path == local) + if (revisionFailures > 0) { + revisionFailures -= 1 + throw WindowsCloudFilesOperationException("open for update", SHARING_VIOLATION) + } events += "revision:${uploaded.remoteRevision}" placeholder = uploaded recordRevision