AppCleaner: Faster deletion when using Root/Shizuku on directories with many files

app-common-io/src/main/aidl/eu/darken/sdmse/common/files/local/ipc/FileOpsConnection.aidl

@@ -21,7 +21,7 @@ interface FileOpsConnection {
 
     boolean exists(in LocalPath path);
 
-    boolean delete(in LocalPath path);
+    boolean delete(in LocalPath path, boolean recursive, boolean dryRun);
 
     RemoteInputStream listFilesStream(in LocalPath path);
 

app-common-io/src/main/java/eu/darken/sdmse/common/files/APathExtensions.kt

@@ -107,13 +107,18 @@ suspend fun <T : APath> T.createDirIfNecessary(gateway: APathGateway<T, out APat
     return this
 }
 
-suspend fun <T : APath> T.delete(gateway: APathGateway<T, out APathLookup<T>, out APathLookupExtended<T>>) {
-    gateway.delete(this)
-    log(VERBOSE) { "APath.delete(): Deleted $this" }
+suspend fun <T : APath> T.delete(
+    gateway: APathGateway<T, out APathLookup<T>, out APathLookupExtended<T>>,
+    recursive: Boolean = false,
+) {
+    gateway.delete(
+        this,
+        recursive = recursive
+    )
+    log(VERBOSE) { "APath.delete(recursive=$recursive): Deleted $this" }
 }
 
-// TODO move this into the gateways?
-suspend fun <T : APath> T.deleteAll(
+suspend fun <T : APath> T.deleteWalk(
     gateway: APathGateway<T, out APathLookup<T>, out APathLookupExtended<T>>,
     filter: (APathLookup<*>) -> Boolean = { true }
 ) {
@@ -122,7 +127,7 @@ suspend fun <T : APath> T.deleteAll(
 
         if (lookup.isDirectory) {
             gateway.listFiles(this).forEach {
-                it.deleteAll(gateway, filter) // Recursion enter
+                it.deleteWalk(gateway, filter) // Recursion enter
             }
         }
 
@@ -141,7 +146,7 @@ suspend fun <T : APath> T.deleteAll(
     }
 
     // Recursion exit
-    this.delete(gateway)
+    this.delete(gateway, recursive = false)
 }
 
 suspend fun <T : APath> T.file(

app-common-io/src/main/java/eu/darken/sdmse/common/files/APathGateway.kt

@@ -54,7 +54,7 @@ interface APathGateway<
 
     suspend fun file(path: P, readWrite: Boolean): FileHandle
 
-    suspend fun delete(path: P)
+    suspend fun delete(path: P, recursive: Boolean)
 
     suspend fun createSymlink(linkPath: P, targetPath: P): Boolean
 

app-common-io/src/main/java/eu/darken/sdmse/common/files/APathLookupExtensions.kt

@@ -1,7 +1,5 @@
 package eu.darken.sdmse.common.files
 
-import eu.darken.sdmse.common.debug.logging.Logging.Priority.VERBOSE
-import eu.darken.sdmse.common.debug.logging.log
 import kotlinx.coroutines.flow.Flow
 import okio.FileHandle
 
@@ -30,16 +28,17 @@ suspend fun <P : APath, PL : APathLookup<P>> PL.exists(
 ): Boolean = lookedUp.exists(gateway)
 
 suspend fun <P : APath, PL : APathLookup<P>, PLE : APathLookupExtended<P>> PL.delete(
-    gateway: APathGateway<P, PL, PLE>
-) {
-    lookedUp.delete(gateway)
-    log(VERBOSE) { "APath.delete(): Deleted $this" }
-}
-
-suspend fun <P : APath, PL : APathLookup<P>> PL.deleteAll(
+    gateway: APathGateway<P, PL, PLE>,
+    recursive: Boolean = false,
+) = lookedUp.delete(
+    gateway,
+    recursive = recursive
+)
+
+suspend fun <P : APath, PL : APathLookup<P>> PL.deletewalk(
     gateway: APathGateway<P, out APathLookup<P>, out APathLookupExtended<P>>,
     filter: (APathLookup<*>) -> Boolean = { true }
-) = lookedUp.deleteAll(gateway, filter)
+) = lookedUp.deleteWalk(gateway, filter)
 
 suspend fun <P : APath, PL : APathLookup<P>> PL.file(
     gateway: APathGateway<P, out APathLookup<P>, out APathLookupExtended<P>>,

app-common-io/src/main/java/eu/darken/sdmse/common/files/GatewaySwitch.kt

@@ -182,8 +182,8 @@ class GatewaySwitch @Inject constructor(
         return useGateway(path) { file(path, readWrite) }
     }
 
-    override suspend fun delete(path: APath) {
-        return useGateway(path) { delete(path) }
+    override suspend fun delete(path: APath, recursive: Boolean) {
+        return useGateway(path) { delete(path, recursive = recursive) }
     }
 
     override suspend fun createSymlink(linkPath: APath, targetPath: APath): Boolean {

app-common-io/src/main/java/eu/darken/sdmse/common/files/local/LocalGateway.kt

@@ -715,9 +715,17 @@ class LocalGateway @Inject constructor(
         }
     }
 
-    override suspend fun delete(path: LocalPath) = delete(path, Mode.AUTO)
+    override suspend fun delete(path: LocalPath, recursive: Boolean) = delete(
+        path,
+        recursive = recursive,
+        mode = Mode.AUTO
+    )
 
-    suspend fun delete(path: LocalPath, mode: Mode = Mode.AUTO): Unit = runIO {
+    suspend fun delete(
+        path: LocalPath,
+        recursive: Boolean = false,
+        mode: Mode = Mode.AUTO
+    ): Unit = runIO {
         try {
             val javaFile = path.asFile()
 
@@ -745,11 +753,16 @@ class LocalGateway @Inject constructor(
                 mode == Mode.NORMAL || mode == Mode.AUTO && normalCanWrite -> {
                     log(TAG, VERBOSE) { "delete($mode->NORMAL): $path" }
 
-                    var success = if (Bugs.isDryRun) {
-                        log(TAG, INFO) { "DRYRUN: Not deleting $javaFile" }
-                        javaFile.canWrite()
-                    } else {
-                        javaFile.delete()
+                    var success = javaFile.run {
+                        when {
+                            Bugs.isDryRun -> {
+                                log(TAG, INFO) { "DRYRUN: Not deleting $javaFile" }
+                                javaFile.canWrite()
+                            }
+
+                            recursive -> deleteRecursively()
+                            else -> delete()
+                        }
                     }
 
                     if (!success) {
@@ -764,7 +777,7 @@ class LocalGateway @Inject constructor(
 
                     if (!success) {
                         if (mode == Mode.AUTO && hasRoot()) {
-                            delete(path, Mode.ROOT)
+                            delete(path, recursive = recursive, mode = Mode.ROOT)
                             return@runIO
                         } else {
                             throw IOException("delete() call returned false")
@@ -773,7 +786,7 @@ class LocalGateway @Inject constructor(
 
                     if (!success) {
                         if (mode == Mode.AUTO && hasShizuku()) {
-                            delete(path, Mode.ADB)
+                            delete(path, recursive = recursive, mode = Mode.ADB)
                             return@runIO
                         } else {
                             throw IOException("delete() call returned false")
@@ -784,12 +797,8 @@ class LocalGateway @Inject constructor(
                 hasRoot() && (mode == Mode.ROOT || mode == Mode.AUTO) -> {
                     log(TAG, VERBOSE) { "delete($mode->ROOT): $path" }
                     rootOps {
-                        var success = if (Bugs.isDryRun) {
-                            log(TAG, INFO) { "DRYRUN: Not deleting (root) $javaFile" }
-                            it.canWrite(path)
-                        } else {
-                            it.delete(path)
-                        }
+                        if (Bugs.isDryRun) log(TAG, INFO) { "DRYRUN: Not deleting (root) $javaFile" }
+                        var success = it.delete(path, recursive = true, dryRun = Bugs.isDryRun)
 
                         if (!success) {
                             // TODO We could move this into the root service for better performance?
@@ -806,12 +815,9 @@ class LocalGateway @Inject constructor(
                 hasShizuku() && (mode == Mode.ADB || mode == Mode.AUTO) -> {
                     log(TAG, VERBOSE) { "delete($mode->ADB): $path" }
                     adbOps {
-                        var success = if (Bugs.isDryRun) {
-                            log(TAG, INFO) { "DRYRUN: Not deleting (adb) $javaFile" }
-                            it.canWrite(path)
-                        } else {
-                            it.delete(path)
-                        }
+                        if (Bugs.isDryRun) log(TAG, INFO) { "DRYRUN: Not deleting (adb) $javaFile" }
+                        var success = it.delete(path, recursive = true, dryRun = Bugs.isDryRun)
+
 
                         if (!success) {
                             // TODO We could move this into the ADB service for better performance?

app-common-io/src/main/java/eu/darken/sdmse/common/files/local/ipc/FileOpsClient.kt

@@ -129,8 +129,8 @@ class FileOpsClient @AssistedInject constructor(
         throw e.unwrapPropagation()
     }
 
-    fun delete(path: LocalPath): Boolean = try {
-        fileOpsConnection.delete(path)
+    fun delete(path: LocalPath, recursive: Boolean, dryRun: Boolean): Boolean = try {
+        fileOpsConnection.delete(path, recursive, dryRun)
     } catch (e: Exception) {
         throw e.unwrapPropagation()
     }

app-common-io/src/main/java/eu/darken/sdmse/common/files/local/ipc/FileOpsHost.kt

@@ -201,11 +201,17 @@ class FileOpsHost @Inject constructor(
         throw e.wrapToPropagate()
     }
 
-    override fun delete(path: LocalPath): Boolean = try {
-        if (Bugs.isTrace) log(TAG, VERBOSE) { "exists($path)..." }
-        path.asFile().delete()
+    override fun delete(path: LocalPath, recursive: Boolean, dryRun: Boolean): Boolean = try {
+        log(TAG, VERBOSE) { "delete($path,recursive=$recursive,dryRun=$dryRun)..." }
+        path.asFile().run {
+            when {
+                dryRun -> canWrite()
+                recursive -> deleteRecursively()
+                else -> delete()
+            }
+        }
     } catch (e: Exception) {
-        log(TAG, ERROR) { "delete(path=$path) failed\n${e.asLog()}" }
+        log(TAG, ERROR) { "delete(path=$path,recursive=$recursive,dryRun=$dryRun) failed\n${e.asLog()}" }
         throw e.wrapToPropagate()
     }
 
@@ -241,12 +247,6 @@ class FileOpsHost @Inject constructor(
         throw e.wrapToPropagate()
     }
 
-    // Not all exception can be passed through the binder
-    // See Parcel.writeException(...)
-    private fun wrapPropagating(e: Exception): Exception {
-        return if (e is RuntimeException) e else RuntimeException(e)
-    }
-
     companion object {
         val TAG = logTag("FileOps", "Service", "Host", Bugs.processTag)
     }

app-common-io/src/main/java/eu/darken/sdmse/common/files/saf/SAFGateway.kt

@@ -158,21 +158,44 @@ class SAFGateway @Inject constructor(
         }
     }
 
-    override suspend fun delete(path: SAFPath) = runIO {
-        try {
-            val docFile = findDocFile(path)
-            log(TAG, VERBOSE) { "delete(): $path -> $docFile" }
+    suspend fun delete(path: SAFPath) = delete(path, recursive = false)
 
-            var success = docFile.delete()
+    override suspend fun delete(path: SAFPath, recursive: Boolean) = runIO {
 
-            if (!success) {
-                success = !docFile.exists
-                if (success) log(TAG, WARN) { "Tried to delete file, but it's already gone: $path" }
-            }
+        log(TAG, VERBOSE) { "delete(recursive=$recursive): $path" }
 
-            if (!success) throw IOException("Document delete() call returned false")
-        } catch (e: Exception) {
-            throw WriteException(path = path, cause = e)
+        val queue = LinkedList(listOf(lookup(path)))
+
+        while (!queue.isEmpty()) {
+            val lookUp = queue.removeFirst()
+
+            if (lookUp.isDirectory) {
+                val newBatch = try {
+                    lookupFiles(lookUp.lookedUp)
+                } catch (e: IOException) {
+                    log(TAG, ERROR) { "Failed to read directory to delete $lookUp: $e" }
+                    throw ReadException(path = path, cause = e)
+                }
+                queue.addAll(newBatch)
+            } else {
+                var success = try {
+                    lookUp.docFile.delete()
+                } catch (e: Exception) {
+                    throw WriteException(path = path, cause = e)
+                }
+
+                if (!success) {
+                    success = try {
+                        !lookUp.docFile.exists
+                    } catch (e: IOException) {
+                        log(TAG, ERROR) { "Failed to perform exists() check $lookUp: $e" }
+                        throw ReadException(path = path, cause = e)
+                    }
+                    if (success) log(TAG, WARN) { "Tried to delete file, but it's already gone: $path" }
+                }
+
+                if (!success) throw IOException("Document delete() call returned false")
+            }
         }
     }
 

app/src/main/java/eu/darken/sdmse/analyzer/core/Analyzer.kt

@@ -27,7 +27,7 @@ import eu.darken.sdmse.common.debug.logging.Logging.Priority.WARN
 import eu.darken.sdmse.common.debug.logging.log
 import eu.darken.sdmse.common.debug.logging.logTag
 import eu.darken.sdmse.common.files.GatewaySwitch
-import eu.darken.sdmse.common.files.deleteAll
+import eu.darken.sdmse.common.files.delete
 import eu.darken.sdmse.common.files.filterDistinctRoots
 import eu.darken.sdmse.common.files.isAncestorOf
 import eu.darken.sdmse.common.files.matches
@@ -188,7 +188,7 @@ class Analyzer @Inject constructor(
             .forEach { target ->
                 log(TAG) { "Deleting $target" }
                 updateProgressSecondary(target.userReadablePath)
-                target.deleteAll(gatewaySwitch)
+                target.delete(gatewaySwitch, recursive = true)
             }
 
         // TODO this seems convoluted, can we come up with a better data pattern?

app/src/main/java/eu/darken/sdmse/appcleaner/core/AppCleaner.kt

@@ -230,9 +230,11 @@ class AppCleaner @Inject constructor(
                         val filter = filters.singleOrNull { it.identifier == filterIdentifier }
                             ?: throw IllegalStateException("Can't find filter for $filterIdentifier")
 
+                        updateProgressSecondary(eu.darken.sdmse.common.R.string.general_progress_loading)
+
                         filter.withProgress(
                             client = this,
-                            onUpdate = { old, new -> old?.copy(secondary = new?.secondary ?: CaString.EMPTY) },
+                            onUpdate = { old, new -> old?.copy(secondary = new?.primary ?: CaString.EMPTY) },
                             onCompletion = { it }
                         ) {
                             val result = process(targets, allMatches)

app/src/main/java/eu/darken/sdmse/appcleaner/core/forensics/BaseExpendablesFilter.kt

@@ -10,13 +10,15 @@ import eu.darken.sdmse.common.debug.logging.logTag
 import eu.darken.sdmse.common.files.APathLookup
 import eu.darken.sdmse.common.files.GatewaySwitch
 import eu.darken.sdmse.common.files.PathException
-import eu.darken.sdmse.common.files.deleteAll
+import eu.darken.sdmse.common.files.delete
 import eu.darken.sdmse.common.files.exists
 import eu.darken.sdmse.common.files.filterDistinctRoots
 import eu.darken.sdmse.common.files.isAncestorOf
 import eu.darken.sdmse.common.flow.throttleLatest
 import eu.darken.sdmse.common.progress.Progress
-import eu.darken.sdmse.common.progress.updateProgressSecondary
+import eu.darken.sdmse.common.progress.increaseProgress
+import eu.darken.sdmse.common.progress.updateProgressCount
+import eu.darken.sdmse.common.progress.updateProgressPrimary
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
 
@@ -35,26 +37,29 @@ abstract class BaseExpendablesFilter : ExpendablesFilter {
         allMatches: Collection<ExpendablesFilter.Match>,
     ): ExpendablesFilter.ProcessResult {
         log(TAG, INFO) { "deleteAll(...) Processing ${targets.size} out of ${allMatches.size} matches" }
+        updateProgressPrimary(eu.darken.sdmse.common.R.string.general_progress_preparing)
         val successful = mutableSetOf<ExpendablesFilter.Match>()
         val failed = mutableSetOf<Pair<ExpendablesFilter.Match, Exception>>()
 
         val distinctRoots = targets.map { it.lookup }.filterDistinctRoots()
 
         if (distinctRoots.size != targets.size) {
             log(TAG, INFO) { "${targets.size} match objects but only ${distinctRoots.size} distinct roots" }
-            if (Bugs.isDebug) {
+            if (Bugs.isTrace) {
                 targets
                     .filter { !distinctRoots.contains(it.lookup) }
                     .forEachIndexed { index, item -> log(TAG, INFO) { "Non distinct root #$index: $item" } }
             }
         }
 
+        updateProgressCount(Progress.Count.Percent(distinctRoots.size))
+
         distinctRoots.forEach { targetRoot ->
-            updateProgressSecondary(targetRoot.userReadablePath)
+            updateProgressPrimary(targetRoot.userReadablePath)
             val main = targets.first { it.lookup == targetRoot }
 
             val mainDeleted = try {
-                targetRoot.deleteAll(gatewaySwitch)
+                targetRoot.delete(gatewaySwitch, recursive = true)
                 log(TAG) { "Main match deleted: $main" }
                 true
             } catch (oge: PathException) {
@@ -76,7 +81,7 @@ abstract class BaseExpendablesFilter : ExpendablesFilter {
 
             // deleteAll(...) starts at leafs, children may have been deleted, even if the top-level dir wasn't
             val affected = allMatches.filter { it != main && main.lookup.isAncestorOf(it.lookup) }
-            if (Bugs.isDebug) {
+            if (Bugs.isTrace) {
                 log(TAG) { "$main affects ${affected.size} other matches" }
                 affected.forEach { log(TAG, VERBOSE) { "Affected: $it" } }
             }
@@ -96,6 +101,7 @@ abstract class BaseExpendablesFilter : ExpendablesFilter {
                     }
                 }
             }
+            increaseProgress()
         }
 
         return ExpendablesFilter.ProcessResult(