mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 03:38:23 +00:00
fix(cordn): recursive delete carries on past a directory it cannot list
deleteRecursivelyQuietly (the okio stand-in for File.deleteRecursively) listed the whole tree with listRecursively(...).toList() inside one try: a single directory that failed to list emptied the list, so nothing was deleted. File.deleteRecursively removed everything it could reach. For the cordn stores that meant a deleted group could keep its plaintext message history, and a migration could merge an imported snapshot into old state. It now lists one directory at a time, so an unlistable directory costs only its own subtree and the result is false. metadataOrNull does not follow symlinks, so a link is still deleted rather than walked into. FileSystemExtTest covers a whole tree, a missing path, a directory that refuses to list (failed before this change: the sibling's messages file survived) and a symlink whose target must survive. Also corrects EncryptedAppendLog's cache-key comment: Path.normalized() does not resolve a relative path against the working directory, unlike File.absolutePath. Every store builds its paths from one directory, so no caller mixes spellings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S7FuNBSKiyVecARSoE4B9P
This commit is contained in:
+5
-1
@@ -100,7 +100,11 @@ class EncryptedAppendLog(
|
||||
var looseEntries: Int,
|
||||
)
|
||||
|
||||
/** Keyed by the normalized path, so two spellings of one file share an entry. */
|
||||
/**
|
||||
* Keyed by the normalized path. That folds `a/./b` and `a/../a/b` together but does not resolve
|
||||
* a relative path against the working directory, so callers must spell one file one way; every
|
||||
* store here builds its paths from one directory.
|
||||
*/
|
||||
private val logs = mutableMapOf<Path, LogState>()
|
||||
|
||||
private fun stateFor(file: Path): LogState =
|
||||
|
||||
+15
-9
@@ -61,16 +61,22 @@ fun FileSystem.deleteQuietly(path: Path): Boolean =
|
||||
* @return true when nothing is left, including when [path] never existed.
|
||||
*/
|
||||
fun FileSystem.deleteRecursivelyQuietly(path: Path): Boolean {
|
||||
// Directories come before their children in this listing, so the reverse
|
||||
// deletes every child before the directory holding it.
|
||||
val children =
|
||||
try {
|
||||
listRecursively(path).toList()
|
||||
} catch (e: IOException) {
|
||||
emptyList()
|
||||
}
|
||||
var deleted = true
|
||||
for (child in children.asReversed()) deleted = deleteQuietly(child) && deleted
|
||||
// metadataOrNull does not follow symlinks, so a link reads as a non-directory
|
||||
// and is deleted itself rather than walked into.
|
||||
if (metadataOrNull(path)?.isDirectory == true) {
|
||||
// Listed one directory at a time, so a directory that cannot be listed
|
||||
// costs only its own subtree: its siblings are still deleted, as
|
||||
// File.deleteRecursively did.
|
||||
val children =
|
||||
try {
|
||||
list(path)
|
||||
} catch (e: IOException) {
|
||||
deleted = false
|
||||
emptyList()
|
||||
}
|
||||
for (child in children) deleted = deleteRecursivelyQuietly(child) && deleted
|
||||
}
|
||||
return deleteQuietly(path) && deleted
|
||||
}
|
||||
|
||||
|
||||
+103
@@ -0,0 +1,103 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.amethyst.commons.util
|
||||
|
||||
import okio.FileSystem
|
||||
import okio.ForwardingFileSystem
|
||||
import okio.IOException
|
||||
import okio.Path
|
||||
import okio.Path.Companion.toOkioPath
|
||||
import java.nio.file.Files
|
||||
import kotlin.test.AfterTest
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertFalse
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
class FileSystemExtTest {
|
||||
private val root = Files.createTempDirectory("fs-ext-test").toFile()
|
||||
|
||||
@AfterTest
|
||||
fun cleanup() {
|
||||
root.deleteRecursively()
|
||||
}
|
||||
|
||||
/** Refuses to list one directory, as an unreadable or concurrently removed one would. */
|
||||
private class UnlistableDirFileSystem(
|
||||
private val unlistable: Path,
|
||||
) : ForwardingFileSystem(FileSystem.SYSTEM) {
|
||||
override fun list(dir: Path): List<Path> {
|
||||
if (dir == unlistable) throw IOException("cannot list $dir")
|
||||
return super.list(dir)
|
||||
}
|
||||
}
|
||||
|
||||
private fun write(
|
||||
relative: String,
|
||||
text: String = "x",
|
||||
) {
|
||||
val file = root.resolve(relative)
|
||||
file.parentFile.mkdirs()
|
||||
file.writeText(text)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun deletesAWholeTree() {
|
||||
write("a/b/c.txt")
|
||||
write("a/d.txt")
|
||||
val dir = root.resolve("a").toOkioPath()
|
||||
|
||||
assertTrue(FileSystem.SYSTEM.deleteRecursivelyQuietly(dir))
|
||||
assertFalse(FileSystem.SYSTEM.exists(dir))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun missingPathCountsAsDeleted() {
|
||||
assertTrue(FileSystem.SYSTEM.deleteRecursivelyQuietly(root.resolve("never").toOkioPath()))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun carriesOnPastADirectoryItCannotList() {
|
||||
// File.deleteRecursively removed everything it could reach and reported false for the
|
||||
// rest. One unlistable subdirectory must not stop its siblings from being deleted.
|
||||
write("g/stuck/inner.txt")
|
||||
write("g/sibling/messages", "plaintext")
|
||||
write("g/state")
|
||||
val dir = root.resolve("g").toOkioPath()
|
||||
val fs = UnlistableDirFileSystem(dir / "stuck")
|
||||
|
||||
assertFalse(fs.deleteRecursivelyQuietly(dir))
|
||||
assertFalse(root.resolve("g/sibling/messages").exists())
|
||||
assertFalse(root.resolve("g/state").exists())
|
||||
assertTrue(root.resolve("g/stuck/inner.txt").exists())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun deletesASymlinkWithoutEmptyingItsTarget() {
|
||||
write("outside/keep.txt")
|
||||
write("h/own.txt")
|
||||
Files.createSymbolicLink(root.resolve("h/link").toPath(), root.resolve("outside").toPath())
|
||||
val dir = root.resolve("h").toOkioPath()
|
||||
|
||||
assertTrue(FileSystem.SYSTEM.deleteRecursivelyQuietly(dir))
|
||||
assertFalse(root.resolve("h").exists())
|
||||
assertTrue(root.resolve("outside/keep.txt").exists())
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user