fix(common-utils): Use the Path API to delete files

Leverage Kotlin's `Path.deleteRecursively()` extension function which on the JVM uses the `Path` API to delete files and directories. That function also does not follow symbolic links on deletion. Fixes #8987. Signed-off-by: Sebastian Schuberth <[email protected]>
oss-review-toolkit · Sep 5, 2024 · a477ded · a477ded
1 parent c8f2baa
commit a477ded
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 19 deletions.
diff --git a/utils/common/src/funTest/kotlin/SafeDeleteRecursivelyFunTest.kt b/utils/common/src/funTest/kotlin/SafeDeleteRecursivelyFunTest.kt
@@ -20,7 +20,6 @@
 package org.ossreviewtoolkit.utils.common
 
 import io.kotest.assertions.throwables.shouldNotThrow
-import io.kotest.assertions.throwables.shouldThrow
 import io.kotest.core.spec.style.WordSpec
 import io.kotest.engine.spec.tempdir
 import io.kotest.matchers.shouldBe
@@ -61,13 +60,11 @@ class SafeDeleteRecursivelyFunTest : WordSpec({
             val nodeDir = tempdir().resolve("node-dir")
             Git().download(pkg, nodeDir)
 
-            // TODO: Fix the implementation so that this does not throw.
-            shouldThrow<IOException> {
+            shouldNotThrow<IOException> {
                 nodeDir.safeDeleteRecursively(force = true)
             }
 
-            // TODO: Expect "false" once the implementation is fixed.
-            nodeDir.exists() shouldBe true
+            nodeDir.exists() shouldBe false
         }
 
         "delete empty parent directories below a base directory" {

diff --git a/utils/common/src/main/kotlin/Extensions.kt b/utils/common/src/main/kotlin/Extensions.kt
@@ -33,6 +33,8 @@ import java.nio.file.attribute.BasicFileAttributes
 import java.util.EnumSet
 import java.util.Locale
 
+import kotlin.io.path.deleteRecursively
+
 /**
  * Call [also] only if the receiver is null, e.g. for error handling, and return the receiver in any case.
  */
@@ -121,19 +123,13 @@ fun File.realFile(): File = toPath().toRealPath().toFile()
  * [baseDirectory] itself is not deleted. Throws an [IOException] if a file could not be deleted.
  */
 fun File.safeDeleteRecursively(force: Boolean = false, baseDirectory: File? = null) {
-    if (isDirectory && !isSymbolicLink()) {
-        Files.newDirectoryStream(toPath()).use { stream ->
-            stream.forEach { path ->
-                path.toFile().safeDeleteRecursively(force)
-            }
-        }
-    }
-
-    if (baseDirectory == this) return
+    // Note that Kotlin's `File.deleteRecursively()` extension function does not work here to delete files with
+    // unmappable characters in their names, so use the `Path.deleteRecursively()` extension function instead.
+    toPath().deleteRecursively()
 
-    if (!delete() && force && setWritable(true)) {
-        // Try again.
-        delete()
+    if (baseDirectory == this) {
+        safeMkdirs()
+        return
     }
 
     if (baseDirectory != null) {
@@ -142,8 +138,6 @@ fun File.safeDeleteRecursively(force: Boolean = false, baseDirectory: File? = nu
             parent = parent.parentFile
         }
     }
-
-    if (exists()) throw IOException("Could not delete file '$absolutePath'.")
 }
 
 /**