diff --git a/AGENTS.md b/AGENTS.md index 12f61458..e58b1269 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,7 +4,11 @@ FileKit is split across multiplatform modules: `filekit-core` contains platform-agnostic APIs, while dialogs, Compose bindings, and Coil integration live in `filekit-dialogs`, `filekit-dialogs-compose`, and `filekit-coil`. Shared source sets live under `src/*Main`, with platform tests in sibling `src/*Test` directories. Sample apps under `samples/` (`sample-core`, `sample-compose`, `sample-file-explorer`) demonstrate integration patterns; update them alongside library changes when user-facing behaviour shifts. API docs and release notes are tracked in `docs/` and `documentation-v0.8.8.md`. ## Build, Test, and Development Commands -Use `./gradlew assemble` to ensure all published artifacts compile before raising a PR. Run `./gradlew :filekit-core:check :filekit-dialogs:check` to execute the multiplatform test matrix for the primary modules. Sample apps can be exercised with `./gradlew :samples:sample-compose:composeApp:run` (desktop) or by opening the Gradle targets in Android Studio for mobile builds. For smoke testing local publishing, run `./gradlew publishToMavenLocal` and consume the artifacts from a sample project. +Keep local validation scoped to the changed module and one relevant target, with `--max-workers=1`; run checks sequentially. For example, use `./gradlew :filekit-core:jvmTest --tests '*PlatformFileDeletionTest*' --max-workers=1` for a deletion regression. + +**Never run repository-wide `./gradlew assemble`, `./gradlew check`, or `./gradlew build` locally**, alone or combined. They overload the maintainer's Mac. Leave broad builds and multiplatform test matrices, including module-level aggregate `check` tasks, to CI; do not use them as a fallback when a targeted check fails. Report the targeted checks run and any validation left to CI. + +Exercise sample apps only when needed for the change. For local publishing smoke tests, scope publishing to the required module and platform instead of publishing every artifact. For Kotlin formatting/linting, run `ktlint '**/*.kt' '**/*.kts' '!**/build/**' -R ktlint-compose-0.4.28-all.jar`. To auto-fix issues, add `--format` to that command. @@ -12,10 +16,10 @@ For Kotlin formatting/linting, run `ktlint '**/*.kt' '**/*.kts' '!**/build/**' - Follow Kotlin official style: four-space indentation, trailing commas where helpful, and `UpperCamelCase` for public APIs. Keep expect/actual implementations mirrored across targets and group platform-specific helpers under the corresponding `src/Main` directory. Compose functions remain PascalCase and should take a `modifier` parameter when rendering UI. Prefer descriptive file names that match the primary type, and keep shared constants in `commonMain` to minimise duplication. ## Testing Guidelines -Add unit tests in the closest `src/Test` directory; default to `commonTest` when behaviour is shared and mirror target-specific coverage otherwise. Test names follow the `Subject_action_expectation` convention (e.g., `FilePicker_openDirectory_returnsFolder`). Run `./gradlew check` locally before every push and ensure new features include regression coverage for at least one non-JVM target. When behaviour depends on native APIs, document manual verification steps in the PR description. +Add unit tests in the closest `src/Test` directory; default to `commonTest` when behaviour is shared and mirror target-specific coverage otherwise. Test names follow the `Subject_action_expectation` convention (e.g., `FilePicker_openDirectory_returnsFolder`). Before pushing, run the relevant targeted checks under the local validation limits above, and ensure new features include regression coverage for at least one non-JVM target. When behaviour depends on native APIs, document manual verification steps in the PR description. ## Commit & Pull Request Guidelines -Commits are short, imperative statements and often begin with an emoji category (e.g., `✨ Add WASM picker`); keep related changes squashed together. Each PR should describe the change, note affected platforms, call out doc updates, and link issues or discussions when relevant. Attach screenshots or screen recordings when UI behaviour changes. Before requesting review, verify CI-critical tasks (`assemble`, `check`), update sample apps if behaviour shifts, and note any follow-up work in the description. +Commits are short, imperative statements and often begin with an emoji category (e.g., `✨ Add WASM picker`); keep related changes squashed together. Each PR should describe the change, note affected platforms, call out doc updates, and link issues or discussions when relevant. Attach screenshots or screen recordings when UI behaviour changes. Before requesting review, report targeted validation and CI status, update sample apps if behaviour shifts, and note any follow-up work in the description. ## Agent skills diff --git a/docs/core/write-file.mdx b/docs/core/write-file.mdx index fb94e5e4..c95b9933 100644 --- a/docs/core/write-file.mdx +++ b/docs/core/write-file.mdx @@ -120,6 +120,19 @@ file.delete() file.delete(mustExist = false) ``` +To delete a directory and its contents, opt into recursive deletion: + +```kotlin +val directory = PlatformFile(FileKit.cacheDir, "temporary-downloads") +directory.delete(recursively = true) +``` + +For filesystem paths, `recursively` defaults to `false`, so deleting a non-empty +directory fails unless you enable it. Symbolic links (including dangling links) +and Windows directory junctions are removed as links; their targets are left +untouched. Android document URIs continue to use the document provider's deletion +behavior. + ## Creating Directories Before writing to a file, you may need to ensure its parent directory exists: diff --git a/filekit-core/src/androidHostTest/kotlin/io/github/vinceglb/filekit/PlatformFileDeletionTest.kt b/filekit-core/src/androidHostTest/kotlin/io/github/vinceglb/filekit/PlatformFileDeletionTest.kt new file mode 100644 index 00000000..7f845ad3 --- /dev/null +++ b/filekit-core/src/androidHostTest/kotlin/io/github/vinceglb/filekit/PlatformFileDeletionTest.kt @@ -0,0 +1,81 @@ +@file:Suppress("ktlint:standard:function-naming", "TestFunctionName") + +package io.github.vinceglb.filekit + +import android.system.ErrnoException +import android.system.Os +import android.system.OsConstants +import android.system.StructStat +import kotlinx.coroutines.test.runTest +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.robolectric.annotation.Implementation +import org.robolectric.annotation.Implements +import java.nio.file.Files +import java.nio.file.LinkOption.NOFOLLOW_LINKS +import java.nio.file.NoSuchFileException +import java.nio.file.Paths +import java.nio.file.attribute.BasicFileAttributes +import kotlin.io.path.createDirectory +import kotlin.io.path.createTempDirectory +import kotlin.io.path.exists +import kotlin.io.path.readText +import kotlin.io.path.writeText +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [23, 36], shadows = [NoFollowOsShadow::class]) +class PlatformFileDeletionTest { + @Test + fun PlatformFile_deleteRecursively_links_areUnlinkedWithoutFollowingTargets() = runTest { + val root = createTempDirectory("filekit-delete-links") + val outside = root.resolve("outside").createDirectory() + val treasure = outside.resolve("treasure.txt") + treasure.writeText("must survive") + val doomed = root.resolve("doomed").createDirectory() + val dangling = Files.createSymbolicLink(doomed.resolve("dangling"), doomed.resolve("missing")) + val link = Files.createSymbolicLink(doomed.resolve("outside"), outside) + try { + PlatformFile(doomed.toFile()).delete(recursively = true) + + assertFalse(doomed.exists(NOFOLLOW_LINKS)) + assertEquals("must survive", treasure.readText()) + } finally { + Files.deleteIfExists(dangling) + Files.deleteIfExists(link) + root.toFile().deleteRecursively() + } + } +} + +// Robolectric's default lstat delegates to stat and follows directory links. Supply the native +// no-follow contract using the host filesystem so this test can catch traversal into a target. +@Implements(Os::class) +class NoFollowOsShadow { + companion object { + @JvmStatic + @Implementation + fun lstat(path: String): StructStat { + val attributes = try { + Files.readAttributes(Paths.get(path), BasicFileAttributes::class.java, NOFOLLOW_LINKS) + } catch (error: NoSuchFileException) { + throw ErrnoException("lstat", OsConstants.ENOENT, error) + } + val mode = when { + attributes.isSymbolicLink -> OsConstants.S_IFLNK + attributes.isDirectory -> OsConstants.S_IFDIR + else -> OsConstants.S_IFREG + } + return StructStat(0, 0, mode, 0, 0, 0, 0, attributes.size(), 0, 0, 0, 0, 0) + } + + @JvmStatic + @Implementation + fun remove(path: String) { + Files.delete(Paths.get(path)) + } + } +} diff --git a/filekit-core/src/androidHostTest/kotlin/io/github/vinceglb/filekit/PlatformFileTestBase.kt b/filekit-core/src/androidHostTest/kotlin/io/github/vinceglb/filekit/PlatformFileTestBase.kt new file mode 100644 index 00000000..4933ee04 --- /dev/null +++ b/filekit-core/src/androidHostTest/kotlin/io/github/vinceglb/filekit/PlatformFileTestBase.kt @@ -0,0 +1,9 @@ +package io.github.vinceglb.filekit + +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [36]) +actual abstract class PlatformFileTestBase actual constructor() diff --git a/filekit-core/src/androidMain/kotlin/io/github/vinceglb/filekit/PlatformFile.android.kt b/filekit-core/src/androidMain/kotlin/io/github/vinceglb/filekit/PlatformFile.android.kt index 4b865392..74b1866e 100644 --- a/filekit-core/src/androidMain/kotlin/io/github/vinceglb/filekit/PlatformFile.android.kt +++ b/filekit-core/src/androidMain/kotlin/io/github/vinceglb/filekit/PlatformFile.android.kt @@ -8,6 +8,9 @@ import android.os.ParcelFileDescriptor import android.provider.DocumentsContract import android.provider.MediaStore import android.provider.OpenableColumns +import android.system.ErrnoException +import android.system.Os +import android.system.OsConstants import android.webkit.MimeTypeMap import androidx.documentfile.provider.DocumentFile import io.github.vinceglb.filekit.exceptions.FileKitException @@ -17,6 +20,7 @@ import io.github.vinceglb.filekit.utils.div import io.github.vinceglb.filekit.utils.toKotlinxPath import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext +import kotlinx.io.IOException import kotlinx.io.RawSink import kotlinx.io.RawSource import kotlinx.io.asSink @@ -449,16 +453,21 @@ private fun PlatformFile.resolveAtomicMoveDestination(source: PlatformFile): Pla return this } -public actual suspend fun PlatformFile.delete(mustExist: Boolean): Unit = +public actual suspend fun PlatformFile.delete(mustExist: Boolean, recursively: Boolean): Unit = withContext(Dispatchers.IO) { when (androidFile) { is AndroidFile.FileWrapper -> { - SystemFileSystem.delete( - path = toKotlinxIoPath(), - mustExist = mustExist, - ) + if (!deleteIfSymbolicLink()) { + if (recursively) deleteChildren() + SystemFileSystem.delete( + path = toKotlinxIoPath(), + mustExist = mustExist, + ) + } } + // No recursion here: SAF has no empty-directory rule to work around. Removing a + // document is the provider's job, and DocumentsContract takes the subtree with it. is AndroidFile.UriWrapper -> { val documentFile = DocumentFile.fromSingleUri(FileKit.context, androidFile.uri) ?: throw FileKitException("Could not access Uri as DocumentFile") @@ -1143,3 +1152,22 @@ private fun Uri.toFileOrNull(): File? { val filePath = path ?: return null return File(filePath) } + +// lstat/remove work on API 21 and operate on the link itself, including a dangling link. +internal actual fun PlatformFile.deleteIfSymbolicLink(): Boolean { + val file = (androidFile as? AndroidFile.FileWrapper)?.file ?: return false + val metadata = try { + Os.lstat(file.absolutePath) + } catch (error: ErrnoException) { + if (error.errno == OsConstants.ENOENT || error.errno == OsConstants.ENOTDIR) return false + throw IOException("Could not inspect ${file.absolutePath}", error) + } + if (!OsConstants.S_ISLNK(metadata.st_mode)) return false + + try { + Os.remove(file.absolutePath) + } catch (error: ErrnoException) { + throw IOException("Could not unlink ${file.absolutePath}", error) + } + return true +} diff --git a/filekit-core/src/appleMain/kotlin/io/github/vinceglb/filekit/PlatformFile.apple.kt b/filekit-core/src/appleMain/kotlin/io/github/vinceglb/filekit/PlatformFile.apple.kt index 75a98c98..09aa181f 100644 --- a/filekit-core/src/appleMain/kotlin/io/github/vinceglb/filekit/PlatformFile.apple.kt +++ b/filekit-core/src/appleMain/kotlin/io/github/vinceglb/filekit/PlatformFile.apple.kt @@ -24,6 +24,7 @@ import kotlinx.cinterop.value import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.IO import kotlinx.coroutines.withContext +import kotlinx.io.IOException import kotlinx.io.files.Path import kotlinx.io.files.SystemFileSystem import kotlinx.serialization.Serializable @@ -39,6 +40,9 @@ import platform.CoreServices.UTTypeCopyPreferredTagWithClass import platform.CoreServices.kUTTagClassMIMEType import platform.Foundation.NSDate import platform.Foundation.NSError +import platform.Foundation.NSFileManager +import platform.Foundation.NSFileType +import platform.Foundation.NSFileTypeSymbolicLink import platform.Foundation.NSLock import platform.Foundation.NSURL import platform.Foundation.NSURLContentModificationDateKey @@ -48,8 +52,11 @@ import platform.Foundation.NSURLResourceKey import platform.Foundation.NSURLTypeIdentifierKey import platform.Foundation.timeIntervalSince1970 import platform.UniformTypeIdentifiers.UTType +import platform.posix.errno import platform.posix.free import platform.posix.realpath +import platform.posix.strerror +import platform.posix.unlink import kotlin.time.ExperimentalTime import kotlin.time.Instant @@ -424,3 +431,17 @@ private fun NSError?.toBookmarkResolutionException(): BookmarkResolutionExceptio reason = classifyAppleBookmarkResolutionError(this), message = "Failed to resolve bookmark data: $this", ) + +// attributesOfItemAtPath does not resolve the link, so a symlink reports its own type here rather +// than the type of whatever it points at. +@OptIn(ExperimentalForeignApi::class) +internal actual fun PlatformFile.deleteIfSymbolicLink(): Boolean { + val path = nsUrl.path ?: return false + val attributes = NSFileManager.defaultManager.attributesOfItemAtPath(path, error = null) + if (attributes?.get(NSFileType) != NSFileTypeSymbolicLink) return false + + if (unlink(path) != 0) { + throw IOException("Could not unlink $path: ${strerror(errno)?.toKString()}") + } + return true +} diff --git a/filekit-core/src/appleTest/kotlin/io/github/vinceglb/filekit/PlatformFileDeletionTest.kt b/filekit-core/src/appleTest/kotlin/io/github/vinceglb/filekit/PlatformFileDeletionTest.kt new file mode 100644 index 00000000..20909f06 --- /dev/null +++ b/filekit-core/src/appleTest/kotlin/io/github/vinceglb/filekit/PlatformFileDeletionTest.kt @@ -0,0 +1,64 @@ +@file:OptIn(kotlinx.cinterop.ExperimentalForeignApi::class) +@file:Suppress("ktlint:standard:function-naming", "TestFunctionName") + +package io.github.vinceglb.filekit + +import kotlinx.coroutines.test.runTest +import kotlinx.io.files.SystemTemporaryDirectory +import platform.posix.symlink +import platform.posix.unlink +import kotlin.random.Random +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse + +class PlatformFileDeletionTest { + @Test + fun PlatformFile_delete_danglingLink_isUnlinked() = runTest { + val root = PlatformFile(SystemTemporaryDirectory) / "filekit-delete-link-${Random.nextInt(0, Int.MAX_VALUE)}" + root.createDirectories() + val link = root / "link" + try { + for (recursively in listOf(false, true)) { + for (mustExist in listOf(false, true)) { + assertEquals(0, symlink("missing", link.path)) + + link.delete(mustExist, recursively) + + assertEquals(emptyList(), root.list(), "The dangling link must be unlinked") + } + } + } finally { + unlink(link.path) + root.delete(mustExist = false) + } + } + + @Test + fun PlatformFile_deleteRecursively_links_areUnlinkedWithoutFollowingTargets() = runTest { + val root = PlatformFile(SystemTemporaryDirectory) / "filekit-delete-tree-${Random.nextInt(0, Int.MAX_VALUE)}" + val outside = root / "outside" + outside.createDirectories() + val treasure = outside / "treasure.txt" + treasure.writeString("must survive") + val doomed = root / "doomed" + doomed.createDirectories() + val links = listOf(doomed / "dangling", doomed / "cycle", doomed / "outside") + try { + assertEquals(0, symlink("missing", links[0].path)) + assertEquals(0, symlink("cycle", links[1].path)) + assertEquals(0, symlink(outside.path, links[2].path)) + + doomed.delete(recursively = true) + + assertFalse(doomed.exists()) + assertEquals("must survive", treasure.readString()) + } finally { + links.forEach { unlink(it.path) } + doomed.delete(mustExist = false) + treasure.delete(mustExist = false) + outside.delete(mustExist = false) + root.delete(mustExist = false) + } + } +} diff --git a/filekit-core/src/jvmAndNativeMain/kotlin/io/github/vinceglb/filekit/PlatformFile.jvmAndNative.kt b/filekit-core/src/jvmAndNativeMain/kotlin/io/github/vinceglb/filekit/PlatformFile.jvmAndNative.kt index 0da7dd8f..cb7c131d 100644 --- a/filekit-core/src/jvmAndNativeMain/kotlin/io/github/vinceglb/filekit/PlatformFile.jvmAndNative.kt +++ b/filekit-core/src/jvmAndNativeMain/kotlin/io/github/vinceglb/filekit/PlatformFile.jvmAndNative.kt @@ -79,10 +79,13 @@ public actual fun PlatformFile.createDirectories(mustCreate: Boolean): Unit = SystemFileSystem.createDirectories(toKotlinxIoPath(), mustCreate) } -public actual suspend fun PlatformFile.delete(mustExist: Boolean): Unit = +public actual suspend fun PlatformFile.delete(mustExist: Boolean, recursively: Boolean): Unit = withScopedAccess { withContext(Dispatchers.IO) { - SystemFileSystem.delete(path = toKotlinxIoPath(), mustExist = mustExist) + if (!deleteIfSymbolicLink()) { + if (recursively) deleteChildren() + SystemFileSystem.delete(path = toKotlinxIoPath(), mustExist = mustExist) + } } } diff --git a/filekit-core/src/jvmMain/kotlin/io/github/vinceglb/filekit/PlatformFile.jvm.kt b/filekit-core/src/jvmMain/kotlin/io/github/vinceglb/filekit/PlatformFile.jvm.kt index d2938987..4cf43b33 100644 --- a/filekit-core/src/jvmMain/kotlin/io/github/vinceglb/filekit/PlatformFile.jvm.kt +++ b/filekit-core/src/jvmMain/kotlin/io/github/vinceglb/filekit/PlatformFile.jvm.kt @@ -1,6 +1,11 @@ package io.github.vinceglb.filekit import com.sun.jna.Platform +import com.sun.jna.platform.win32.Kernel32 +import com.sun.jna.platform.win32.WinBase.INVALID_FILE_ATTRIBUTES +import com.sun.jna.platform.win32.WinError.ERROR_FILE_NOT_FOUND +import com.sun.jna.platform.win32.WinError.ERROR_PATH_NOT_FOUND +import com.sun.jna.platform.win32.WinNT.FILE_ATTRIBUTE_REPARSE_POINT import io.github.vinceglb.filekit.exceptions.BookmarkResolutionException import io.github.vinceglb.filekit.exceptions.BookmarkResolutionFailure import io.github.vinceglb.filekit.mimeType.MimeType @@ -8,6 +13,7 @@ import io.github.vinceglb.filekit.utils.toFile import io.github.vinceglb.filekit.utils.toKotlinxIoPath import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext +import kotlinx.io.IOException import kotlinx.io.files.Path import kotlinx.io.files.SystemFileSystem import kotlinx.serialization.Serializable @@ -180,3 +186,23 @@ public actual fun PlatformFile.Companion.resolveBookmarkData( shouldRefresh = Platform.isMac(), ) } + +internal actual fun PlatformFile.deleteIfSymbolicLink(): Boolean { + val isLink = if (Platform.isWindows()) { + // Files.isSymbolicLink excludes directory junctions. Inspect the entry's reparse attribute. + val attributes = Kernel32.INSTANCE.GetFileAttributes(file.absolutePath) + if (attributes == INVALID_FILE_ATTRIBUTES) { + val error = Kernel32.INSTANCE.GetLastError() + if (error == ERROR_FILE_NOT_FOUND || error == ERROR_PATH_NOT_FOUND) return false + throw IOException("Could not inspect ${file.absolutePath}: Windows error $error") + } + (attributes and FILE_ATTRIBUTE_REPARSE_POINT) != 0 + } else { + Files.isSymbolicLink(file.toPath()) + } + if (!isLink) return false + + // Unlike kotlinx-io's delete, this does not check whether the link's target exists first. + Files.delete(file.toPath()) + return true +} diff --git a/filekit-core/src/jvmTest/kotlin/io/github/vinceglb/filekit/PlatformFileDeletionTest.kt b/filekit-core/src/jvmTest/kotlin/io/github/vinceglb/filekit/PlatformFileDeletionTest.kt new file mode 100644 index 00000000..2335e7f7 --- /dev/null +++ b/filekit-core/src/jvmTest/kotlin/io/github/vinceglb/filekit/PlatformFileDeletionTest.kt @@ -0,0 +1,80 @@ +@file:Suppress("ktlint:standard:function-naming", "TestFunctionName") + +package io.github.vinceglb.filekit + +import com.sun.jna.Platform +import kotlinx.coroutines.test.runTest +import org.junit.Assume.assumeTrue +import java.nio.file.Files +import java.nio.file.LinkOption.NOFOLLOW_LINKS +import kotlin.io.path.createDirectory +import kotlin.io.path.createTempDirectory +import kotlin.io.path.exists +import kotlin.io.path.readText +import kotlin.io.path.writeText +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse + +class PlatformFileDeletionTest { + @Test + fun PlatformFile_delete_danglingLink_isUnlinked() = runTest { + val root = createTempDirectory("filekit-delete-link") + val link = root.resolve("link") + try { + for (recursively in listOf(false, true)) { + for (mustExist in listOf(false, true)) { + Files.createSymbolicLink(link, root.resolve("missing")) + + PlatformFile(link.toFile()).delete(mustExist, recursively) + + assertFalse(link.exists(NOFOLLOW_LINKS), "The dangling link must be unlinked") + } + } + } finally { + Files.deleteIfExists(link) + Files.delete(root) + } + } + + @Test + fun PlatformFile_deleteRecursively_danglingLink_removesDirectory() = runTest { + val root = createTempDirectory("filekit-delete-dangling") + val link = Files.createSymbolicLink(root.resolve("link"), root.resolve("missing")) + try { + PlatformFile(root.toFile()).delete(recursively = true) + + assertFalse(root.exists(NOFOLLOW_LINKS)) + } finally { + Files.deleteIfExists(link) + Files.deleteIfExists(root) + } + } + + @Test + fun PlatformFile_deleteRecursively_windowsJunction_preservesTarget() = runTest { + assumeTrue("Directory junctions are specific to Windows", Platform.isWindows()) + val root = createTempDirectory("filekit-delete-junction") + val outside = root.resolve("outside").createDirectory() + val treasure = outside.resolve("treasure.txt") + treasure.writeText("must survive") + val doomed = root.resolve("doomed").createDirectory() + val junction = doomed.resolve("junction") + try { + val process = ProcessBuilder("cmd", "/c", "mklink", "/J", junction.toString(), outside.toString()) + .redirectErrorStream(true) + .start() + val output = process.inputStream.bufferedReader().use { it.readText() } + assertEquals(0, process.waitFor(), "Could not create a junction: $output") + + PlatformFile(doomed.toFile()).delete(recursively = true) + + assertFalse(doomed.exists(NOFOLLOW_LINKS)) + assertEquals("must survive", treasure.readText()) + } finally { + // Remove the junction itself before cleaning up; never traverse it, even on failure. + Files.deleteIfExists(junction) + root.toFile().deleteRecursively() + } + } +} diff --git a/filekit-core/src/jvmTest/kotlin/io/github/vinceglb/filekit/PlatformFileJvmTest.kt b/filekit-core/src/jvmTest/kotlin/io/github/vinceglb/filekit/PlatformFileJvmTest.kt index d719d23e..cadb68a2 100644 --- a/filekit-core/src/jvmTest/kotlin/io/github/vinceglb/filekit/PlatformFileJvmTest.kt +++ b/filekit-core/src/jvmTest/kotlin/io/github/vinceglb/filekit/PlatformFileJvmTest.kt @@ -10,14 +10,39 @@ import io.github.vinceglb.filekit.mimeType.MimeType import kotlinx.coroutines.test.runTest import kotlinx.io.files.Path import java.io.File +import java.nio.file.Files import kotlin.coroutines.Continuation +import kotlin.io.path.createDirectory import kotlin.io.path.createTempDirectory +import kotlin.io.path.exists +import kotlin.io.path.writeText import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith import kotlin.test.assertFalse +import kotlin.test.assertTrue class PlatformFileJvmTest { + @Test + fun PlatformFile_deleteRecursively_unlinksSymlinkAndLeavesItsTargetIntact() = runTest { + val tempRoot = createTempDirectory("filekit-symlink-test") + try { + val outside = tempRoot.resolve("outside").createDirectory() + val treasure = outside.resolve("treasure.txt") + treasure.writeText("must survive") + + val doomed = tempRoot.resolve("doomed").createDirectory() + Files.createSymbolicLink(doomed.resolve("link-to-outside"), outside) + + PlatformFile(doomed.toFile()).delete(recursively = true) + + assertFalse(doomed.exists(), "the tree the caller asked to remove is gone") + assertTrue(treasure.exists(), "the symlink target lives outside that tree and is untouched") + } finally { + tempRoot.toFile().deleteRecursively() + } + } + private val resourceDirectory = PlatformFile(Path("src/nonWebTest/resources")) private val textFile = PlatformFile(resourceDirectory, "hello.txt") private val imageFile = PlatformFile(resourceDirectory, "compose-logo.png") diff --git a/filekit-core/src/jvmTest/kotlin/io/github/vinceglb/filekit/PlatformFileTestBase.kt b/filekit-core/src/jvmTest/kotlin/io/github/vinceglb/filekit/PlatformFileTestBase.kt new file mode 100644 index 00000000..220eef57 --- /dev/null +++ b/filekit-core/src/jvmTest/kotlin/io/github/vinceglb/filekit/PlatformFileTestBase.kt @@ -0,0 +1,3 @@ +package io.github.vinceglb.filekit + +actual abstract class PlatformFileTestBase actual constructor() diff --git a/filekit-core/src/linuxMain/kotlin/io/github/vinceglb/filekit/PlatformFile.linux.kt b/filekit-core/src/linuxMain/kotlin/io/github/vinceglb/filekit/PlatformFile.linux.kt index 99e91cd9..1b4e7b5e 100644 --- a/filekit-core/src/linuxMain/kotlin/io/github/vinceglb/filekit/PlatformFile.linux.kt +++ b/filekit-core/src/linuxMain/kotlin/io/github/vinceglb/filekit/PlatformFile.linux.kt @@ -12,14 +12,23 @@ import kotlinx.cinterop.toKString import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.IO import kotlinx.coroutines.withContext +import kotlinx.io.IOException import kotlinx.io.buffered import kotlinx.io.files.Path import kotlinx.io.files.SystemFileSystem import kotlinx.io.readString import kotlinx.serialization.Serializable +import platform.posix.ENOENT +import platform.posix.ENOTDIR +import platform.posix.S_IFLNK +import platform.posix.S_IFMT +import platform.posix.errno import platform.posix.fnmatch import platform.posix.getcwd +import platform.posix.lstat import platform.posix.stat +import platform.posix.strerror +import platform.posix.unlink import kotlin.time.ExperimentalTime import kotlin.time.Instant @@ -295,3 +304,20 @@ public actual fun PlatformFile.Companion.resolveBookmarkData( shouldRefresh = false, ) } + +// lstat, not stat: stat would resolve the link and report on its target. +@OptIn(ExperimentalForeignApi::class) +internal actual fun PlatformFile.deleteIfSymbolicLink(): Boolean = memScoped { + val path = absolutePath() + val statBuf = alloc() + if (lstat(path, statBuf.ptr) != 0) { + if (errno == ENOENT || errno == ENOTDIR) return@memScoped false + throw IOException("Could not inspect $path: ${strerror(errno)?.toKString()}") + } + if ((statBuf.st_mode.toInt() and S_IFMT) != S_IFLNK) return@memScoped false + + if (unlink(path) != 0) { + throw IOException("Could not unlink $path: ${strerror(errno)?.toKString()}") + } + true +} diff --git a/filekit-core/src/mingwX64Main/kotlin/io/github/vinceglb/filekit/PlatformFile.mingw.kt b/filekit-core/src/mingwX64Main/kotlin/io/github/vinceglb/filekit/PlatformFile.mingw.kt index fc0fad6c..b05fbb9e 100644 --- a/filekit-core/src/mingwX64Main/kotlin/io/github/vinceglb/filekit/PlatformFile.mingw.kt +++ b/filekit-core/src/mingwX64Main/kotlin/io/github/vinceglb/filekit/PlatformFile.mingw.kt @@ -14,21 +14,31 @@ import kotlinx.cinterop.value import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.IO import kotlinx.coroutines.withContext +import kotlinx.io.IOException import kotlinx.io.files.Path import kotlinx.io.files.SystemFileSystem import kotlinx.serialization.Serializable import platform.windows.DWORDVar +import platform.windows.DeleteFileW +import platform.windows.ERROR_FILE_NOT_FOUND +import platform.windows.ERROR_PATH_NOT_FOUND import platform.windows.FILETIME +import platform.windows.FILE_ATTRIBUTE_DIRECTORY +import platform.windows.FILE_ATTRIBUTE_REPARSE_POINT import platform.windows.GET_FILEEX_INFO_LEVELS import platform.windows.GetFileAttributesExW +import platform.windows.GetFileAttributesW import platform.windows.GetFullPathNameW +import platform.windows.GetLastError import platform.windows.HKEYVar import platform.windows.HKEY_CLASSES_ROOT +import platform.windows.INVALID_FILE_ATTRIBUTES import platform.windows.KEY_READ import platform.windows.MAX_PATH import platform.windows.RegCloseKey import platform.windows.RegOpenKeyExW import platform.windows.RegQueryValueExW +import platform.windows.RemoveDirectoryW import platform.windows.WIN32_FILE_ATTRIBUTE_DATA import kotlin.time.ExperimentalTime import kotlin.time.Instant @@ -230,3 +240,27 @@ private fun FILETIME.toInstant(): Instant { val epochMillis = (windowsTicks / 10_000L) - 11_644_473_600_000L return Instant.fromEpochMilliseconds(epochMillis) } + +// Windows models symlinks and junctions as reparse points, and GetFileAttributesW reports on the +// entry itself rather than on whatever it redirects to. +@OptIn(ExperimentalForeignApi::class) +internal actual fun PlatformFile.deleteIfSymbolicLink(): Boolean { + val path = absolutePath() + val attributes = GetFileAttributesW(path) + if (attributes == INVALID_FILE_ATTRIBUTES) { + val error = GetLastError() + if (error == ERROR_FILE_NOT_FOUND.toUInt() || error == ERROR_PATH_NOT_FOUND.toUInt()) return false + throw IOException("Could not inspect $path: Windows error $error") + } + if ((attributes and FILE_ATTRIBUTE_REPARSE_POINT.toUInt()) == 0u) return false + + val removed = if ((attributes and FILE_ATTRIBUTE_DIRECTORY.toUInt()) != 0u) { + RemoveDirectoryW(path) + } else { + DeleteFileW(path) + } + if (removed == 0) { + throw IOException("Could not unlink $path: Windows error ${GetLastError()}") + } + return true +} diff --git a/filekit-core/src/nativeTest/kotlin/io/github/vinceglb/filekit/PlatformFileTestBase.kt b/filekit-core/src/nativeTest/kotlin/io/github/vinceglb/filekit/PlatformFileTestBase.kt new file mode 100644 index 00000000..220eef57 --- /dev/null +++ b/filekit-core/src/nativeTest/kotlin/io/github/vinceglb/filekit/PlatformFileTestBase.kt @@ -0,0 +1,3 @@ +package io.github.vinceglb.filekit + +actual abstract class PlatformFileTestBase actual constructor() diff --git a/filekit-core/src/nonWebMain/kotlin/io/github/vinceglb/filekit/PlatformFile.nonWeb.kt b/filekit-core/src/nonWebMain/kotlin/io/github/vinceglb/filekit/PlatformFile.nonWeb.kt index e1e6c115..9e11051b 100644 --- a/filekit-core/src/nonWebMain/kotlin/io/github/vinceglb/filekit/PlatformFile.nonWeb.kt +++ b/filekit-core/src/nonWebMain/kotlin/io/github/vinceglb/filekit/PlatformFile.nonWeb.kt @@ -216,8 +216,35 @@ public expect suspend fun PlatformFile.atomicMove(destination: PlatformFile) * Deletes this file. * * @param mustExist If `true`, fails if the file does not exist. Defaults to `true`. + * @param recursively If `true`, a directory is emptied before it is removed. Defaults to `false`, + * which fails on a filesystem directory that still has contents. Symbolic links (including dangling + * links) and Windows directory junctions are unlinked, never followed. Android document URI deletion + * is handled by the document provider regardless of this flag. */ -public expect suspend fun PlatformFile.delete(mustExist: Boolean = true) +public expect suspend fun PlatformFile.delete( + mustExist: Boolean = true, + recursively: Boolean = false, +) + +/** + * Empties this directory, depth first, leaving the directory itself in place. Does nothing when + * this is not a directory. + * + * The caller must handle links with [deleteIfSymbolicLink] before entering this function. + */ +internal suspend fun PlatformFile.deleteChildren() { + if (!isDirectory()) return + list().forEach { child -> + child.delete(mustExist = false, recursively = true) + } +} + +/** + * Unlinks this entry and returns `true` if it is a symbolic link or a Windows reparse point. + * Returns `false` for other entries and missing paths. Inspection and deletion must not follow the + * target: it can be missing, cyclic, or outside the tree being deleted. Deletion failures propagate. + */ +internal expect fun PlatformFile.deleteIfSymbolicLink(): Boolean /** * Appends a child path to this [PlatformFile]. diff --git a/filekit-core/src/nonWebTest/kotlin/io/github/vinceglb/filekit/PlatformFileNonWebTest.kt b/filekit-core/src/nonWebTest/kotlin/io/github/vinceglb/filekit/PlatformFileNonWebTest.kt index da9c2654..7445d41a 100644 --- a/filekit-core/src/nonWebTest/kotlin/io/github/vinceglb/filekit/PlatformFileNonWebTest.kt +++ b/filekit-core/src/nonWebTest/kotlin/io/github/vinceglb/filekit/PlatformFileNonWebTest.kt @@ -15,7 +15,7 @@ import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertTrue -class PlatformFileNonWebTest { +class PlatformFileNonWebTest : PlatformFileTestBase() { private val resourceDirectory = FileKit.projectDir / "src/nonWebTest/resources" private val textFile = resourceDirectory / "hello.txt" private val imageFile = resourceDirectory / "compose-logo.png" @@ -109,6 +109,52 @@ class PlatformFileNonWebTest { PlatformFile("").delete(mustExist = false) } + @Test + fun PlatformFile_deleteNonEmptyDirectory_fails() = runTest { + val root = resourceDirectory / "delete-non-recursive" + try { + (root / "nested").createDirectories() + (root / "nested" / "leaf.txt").writeString("leaf") + + assertFailsWith { root.delete() } + assertTrue(root.exists()) + } finally { + root.delete(mustExist = false, recursively = true) + } + } + + @Test + fun PlatformFile_deleteNonEmptyDirectoryRecursively_removesWholeTree() = runTest { + val root = resourceDirectory / "delete-recursive" + try { + (root / "a" / "b").createDirectories() + (root / "a" / "b" / "leaf.txt").writeString("leaf") + (root / "a" / "sibling.txt").writeString("sibling") + (root / "top.txt").writeString("top") + + root.delete(recursively = true) + + assertFalse(root.exists()) + } finally { + root.delete(mustExist = false, recursively = true) + } + } + + @Test + fun PlatformFile_deleteFileRecursively_removesFile() = runTest { + val file = resourceDirectory / "delete-recursive-file.txt" + file.writeString("content") + + file.delete(recursively = true) + + assertFalse(file.exists()) + } + + @Test + fun PlatformFile_deleteMissingPathRecursivelyWithMustExistFalse_doesNothing() = runTest { + (resourceDirectory / "never-created-directory").delete(mustExist = false, recursively = true) + } + @Test fun testPlatformFileReadBytes() = runTest { val textFileContent = textFile.readString() diff --git a/filekit-core/src/nonWebTest/kotlin/io/github/vinceglb/filekit/PlatformFileTestBase.kt b/filekit-core/src/nonWebTest/kotlin/io/github/vinceglb/filekit/PlatformFileTestBase.kt new file mode 100644 index 00000000..7bd16940 --- /dev/null +++ b/filekit-core/src/nonWebTest/kotlin/io/github/vinceglb/filekit/PlatformFileTestBase.kt @@ -0,0 +1,4 @@ +package io.github.vinceglb.filekit + +// Android file operations call framework APIs and need a Robolectric runner in host tests. +expect abstract class PlatformFileTestBase()