Core: Support seekable file sources via Okio.FileHandle

Replace the previous sepperate read/write functions. Allows `Coil` to efficiently load video thumbnails. Improves SAF performance. See #1434 See coil-kt/coil#2550
d4rken-org · Oct 18, 2024 · f12f14b · f12f14b
1 parent 9e0f931
commit f12f14b
Show file tree

Hide file tree

Showing 19 changed files with 363 additions and 167 deletions.
diff --git a/app-common-io/src/main/aidl/eu/darken/sdmse/common/files/local/ipc/FileOpsConnection.aidl b/app-common-io/src/main/aidl/eu/darken/sdmse/common/files/local/ipc/FileOpsConnection.aidl
@@ -1,5 +1,6 @@
 package eu.darken.sdmse.common.files.local.ipc;
 
+import eu.darken.sdmse.common.ipc.RemoteFileHandle;
 import eu.darken.sdmse.common.ipc.RemoteInputStream;
 import eu.darken.sdmse.common.ipc.RemoteOutputStream;
 import eu.darken.sdmse.common.files.local.LocalPath;
@@ -10,8 +11,7 @@ import eu.darken.sdmse.common.files.Permissions;
 
 interface FileOpsConnection {
 
-    RemoteInputStream readFile(in LocalPath path);
-    RemoteOutputStream writeFile(in LocalPath path);
+    RemoteFileHandle file(in LocalPath path, boolean readWrite);
 
     boolean mkdirs(in LocalPath path);
     boolean createNewFile(in LocalPath path);

diff --git a/app-common-io/src/main/aidl/eu/darken/sdmse/common/ipc/RemoteFileHandle.aidl b/app-common-io/src/main/aidl/eu/darken/sdmse/common/ipc/RemoteFileHandle.aidl
@@ -0,0 +1,12 @@
+package eu.darken.sdmse.common.ipc;
+
+interface RemoteFileHandle {
+    boolean readWrite();
+    // We need inout for the buffer, as FileHandle will pass partially filled buffers that need to stay partially filled when returned
+    int read(long fileOffset, inout byte[] buffer, int bufferOffset, int byteCount);
+    void write(long fileOffset, inout byte[] buffer, int bufferOffset, int byteCount);
+    void resize(long size);
+    long size();
+    void flush();
+    void close();
+}
diff --git a/app-common-io/src/main/java/eu/darken/sdmse/common/files/APathExtensions.kt b/app-common-io/src/main/java/eu/darken/sdmse/common/files/APathExtensions.kt
@@ -14,8 +14,7 @@ import eu.darken.sdmse.common.files.saf.isAncestorOf
 import eu.darken.sdmse.common.files.saf.isParentOf
 import eu.darken.sdmse.common.files.saf.startsWith
 import kotlinx.coroutines.flow.Flow
-import okio.Sink
-import okio.Source
+import okio.FileHandle
 import java.io.File
 import java.io.IOException
 import java.time.Instant
@@ -145,12 +144,11 @@ suspend fun <T : APath> T.deleteAll(
     this.delete(gateway)
 }
 
-suspend fun <T : APath> T.write(gateway: APathGateway<T, out APathLookup<T>, out APathLookupExtended<T>>): Sink {
-    return gateway.write(this)
-}
-
-suspend fun <T : APath> T.read(gateway: APathGateway<T, out APathLookup<T>, out APathLookupExtended<T>>): Source {
-    return gateway.read(this)
+suspend fun <T : APath> T.file(
+    gateway: APathGateway<T, out APathLookup<T>, out APathLookupExtended<T>>,
+    readWrite: Boolean,
+): FileHandle {
+    return gateway.file(this, readWrite)
 }
 
 suspend fun <T : APath> T.createSymlink(

diff --git a/app-common-io/src/main/java/eu/darken/sdmse/common/files/APathGateway.kt b/app-common-io/src/main/java/eu/darken/sdmse/common/files/APathGateway.kt
@@ -2,8 +2,7 @@ package eu.darken.sdmse.common.files
 
 import eu.darken.sdmse.common.sharedresource.HasSharedResource
 import kotlinx.coroutines.flow.Flow
-import okio.Sink
-import okio.Source
+import okio.FileHandle
 import java.time.Instant
 
 interface APathGateway<
@@ -53,9 +52,7 @@ interface APathGateway<
 
     suspend fun canRead(path: P): Boolean
 
-    suspend fun read(path: P): Source
-
-    suspend fun write(path: P): Sink
+    suspend fun file(path: P, readWrite: Boolean): FileHandle
 
     suspend fun delete(path: P)
 

diff --git a/app-common-io/src/main/java/eu/darken/sdmse/common/files/APathLookupExtensions.kt b/app-common-io/src/main/java/eu/darken/sdmse/common/files/APathLookupExtensions.kt
@@ -3,8 +3,7 @@ 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.Sink
-import okio.Source
+import okio.FileHandle
 
 
 val APathLookup<*>.isDirectory: Boolean
@@ -42,13 +41,10 @@ suspend fun <P : APath, PL : APathLookup<P>> PL.deleteAll(
     filter: (APathLookup<*>) -> Boolean = { true }
 ) = lookedUp.deleteAll(gateway, filter)
 
-suspend fun <P : APath, PL : APathLookup<P>> PL.write(
-    gateway: APathGateway<P, out APathLookup<P>, out APathLookupExtended<P>>
-): Sink = lookedUp.write(gateway)
-
-suspend fun <P : APath, PL : APathLookup<P>> PL.read(
-    gateway: APathGateway<P, out APathLookup<P>, out APathLookupExtended<P>>
-): Source = lookedUp.read(gateway)
+suspend fun <P : APath, PL : APathLookup<P>> PL.file(
+    gateway: APathGateway<P, out APathLookup<P>, out APathLookupExtended<P>>,
+    readWrite: Boolean
+): FileHandle = lookedUp.file(gateway, readWrite)
 
 suspend fun <P : APath, PL : APathLookup<P>> PL.canRead(
     gateway: APathGateway<P, out APathLookup<P>, out APathLookupExtended<P>>

diff --git a/app-common-io/src/main/java/eu/darken/sdmse/common/files/FileHandleWithCallbacks.kt b/app-common-io/src/main/java/eu/darken/sdmse/common/files/FileHandleWithCallbacks.kt
@@ -0,0 +1,39 @@
+package eu.darken.sdmse.common.files
+
+import okio.FileHandle
+
+class FileHandleWithCallbacks(
+    private val wrapped: FileHandle,
+    private val onPostClosed: () -> Unit
+) : FileHandle(wrapped.readWrite) {
+
+    override fun protectedFlush() = wrapped.flush()
+
+    override fun protectedResize(size: Long) = wrapped.resize(size)
+
+    override fun protectedSize(): Long = wrapped.size()
+
+    override fun protectedRead(
+        fileOffset: Long,
+        array: ByteArray,
+        arrayOffset: Int,
+        byteCount: Int
+    ): Int = wrapped.read(fileOffset, array, arrayOffset, byteCount)
+
+    override fun protectedWrite(
+        fileOffset: Long,
+        array: ByteArray,
+        arrayOffset: Int,
+        byteCount: Int
+    ) = wrapped.write(fileOffset, array, arrayOffset, byteCount)
+
+    override fun protectedClose() {
+        try {
+            wrapped.close()
+        } finally {
+            onPostClosed()
+        }
+    }
+}
+
+fun FileHandle.callbacks(onPostClosed: () -> Unit): FileHandle = FileHandleWithCallbacks(this, onPostClosed)
diff --git a/app-common-io/src/main/java/eu/darken/sdmse/common/files/GatewaySwitch.kt b/app-common-io/src/main/java/eu/darken/sdmse/common/files/GatewaySwitch.kt
@@ -16,9 +16,8 @@ import eu.darken.sdmse.common.storage.PathMapper
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.plus
+import okio.FileHandle
 import okio.IOException
-import okio.Sink
-import okio.Source
 import java.time.Instant
 import javax.inject.Inject
 import javax.inject.Singleton
@@ -179,12 +178,8 @@ class GatewaySwitch @Inject constructor(
         return useGateway(path) { canRead(path) }
     }
 
-    override suspend fun read(path: APath): Source {
-        return useGateway(path) { read(path) }
-    }
-
-    override suspend fun write(path: APath): Sink {
-        return useGateway(path) { write(path) }
+    override suspend fun file(path: APath, readWrite: Boolean): FileHandle {
+        return useGateway(path) { file(path, readWrite) }
     }
 
     override suspend fun delete(path: APath) {

diff --git a/app-common-io/src/main/java/eu/darken/sdmse/common/files/local/LocalGateway.kt b/app-common-io/src/main/java/eu/darken/sdmse/common/files/local/LocalGateway.kt
@@ -23,6 +23,7 @@ import eu.darken.sdmse.common.files.core.local.parentsInclusive
 import eu.darken.sdmse.common.files.local.ipc.FileOpsClient
 import eu.darken.sdmse.common.funnel.IPCFunnel
 import eu.darken.sdmse.common.hasApiLevel
+import eu.darken.sdmse.common.ipc.fileHandle
 import eu.darken.sdmse.common.pkgs.pkgops.LibcoreTool
 import eu.darken.sdmse.common.root.RootManager
 import eu.darken.sdmse.common.root.canUseRootNow
@@ -38,10 +39,7 @@ import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.onCompletion
 import kotlinx.coroutines.plus
 import kotlinx.coroutines.withContext
-import okio.Sink
-import okio.Source
-import okio.sink
-import okio.source
+import okio.FileHandle
 import java.io.File
 import java.io.IOException
 import java.time.Instant
@@ -664,82 +662,56 @@ class LocalGateway @Inject constructor(
         }
     }
 
-    override suspend fun read(path: LocalPath): Source = read(path, Mode.AUTO)
+    override suspend fun file(path: LocalPath, readWrite: Boolean): FileHandle = file(path, readWrite, Mode.AUTO)
 
-    suspend fun read(path: LocalPath, mode: Mode = Mode.AUTO): Source = runIO {
+    suspend fun file(path: LocalPath, readWrite: Boolean, mode: Mode = Mode.AUTO): FileHandle = runIO {
         try {
-            val javaFile = path.asFile()
+            val file = path.asFile()
             val canNormalOpen = when (mode) {
                 Mode.ROOT -> false
                 Mode.ADB -> false
-                else -> javaFile.isReadable()
-            }
-
-            when {
-                mode == Mode.NORMAL || mode == Mode.AUTO && canNormalOpen -> {
-                    log(TAG, VERBOSE) { "read($mode->NORMAL): $path" }
-                    javaFile.source()
-                }
-
-                hasRoot() && (mode == Mode.ROOT || mode == Mode.AUTO) -> {
-                    log(TAG, VERBOSE) { "read($mode->ROOT): $path" }
-                    // We need to keep the resource alive until the caller is done with the Source object
-                    val resource = rootManager.serviceClient.get()
-                    rootOps { it.readFile(path).callbacks { resource.close() } }
-                }
-
-                hasShizuku() && (mode == Mode.ADB || mode == Mode.AUTO) -> {
-                    log(TAG, VERBOSE) { "read($mode->ADB): $path" }
-                    // We need to keep the resource alive until the caller is done with the Source object
-                    val resource = shizukuManager.serviceClient.get()
-                    adbOps { it.readFile(path).callbacks { resource.close() } }
+                else -> when {
+                    readWrite -> (file.exists() && file.canWrite()) || !file.exists() && file.parentFile?.canWrite() == true
+                    else -> file.isReadable()
                 }
-
-                else -> throw IOException("No matching mode available.")
-            }
-        } catch (e: IOException) {
-            log(TAG, WARN) { "read(path=$path, mode=$mode) failed." }
-            throw ReadException(path = path, cause = e)
-        }
-    }
-
-    override suspend fun write(path: LocalPath): Sink = write(path, Mode.AUTO)
-
-    suspend fun write(path: LocalPath, mode: Mode = Mode.AUTO): Sink = runIO {
-        try {
-            val file = path.asFile()
-
-            val canOpen = when (mode) {
-                Mode.ROOT -> false
-                Mode.ADB -> false
-                else -> (file.exists() && file.canWrite()) || !file.exists() && file.parentFile?.canWrite() == true
             }
 
             when {
-                mode == Mode.NORMAL || mode == Mode.AUTO && canOpen -> {
-                    log(TAG, VERBOSE) { "write($mode->NORMAL): $path" }
-                    file.sink()
+                mode == Mode.NORMAL || mode == Mode.AUTO && canNormalOpen -> {
+                    log(TAG, VERBOSE) { "file($mode->NORMAL): $path" }
+                    file.fileHandle(readWrite)
                 }
 
                 hasRoot() && (mode == Mode.ROOT || mode == Mode.AUTO) -> {
-                    log(TAG, VERBOSE) { "write($mode->ROOT): $path" }
-                    // We need to keep the resource alive until the caller is done with the Sink object
+                    log(TAG, VERBOSE) { "file($mode->ROOT, RW=$readWrite): $path" }
+                    // We need to keep the resource alive until the caller is done with the object
                     val resource = rootManager.serviceClient.get()
-                    rootOps { it.writeFile(path).callbacks { resource.close() } }
+                    rootOps {
+                        it.file(path, readWrite).callbacks {
+                            resource.close()
+                            log(TAG, VERBOSE) { "file($mode->ROOT, RW=$readWrite): Closing resource for $path" }
+                        }
+                    }
                 }
 
                 hasShizuku() && (mode == Mode.ADB || mode == Mode.AUTO) -> {
-                    log(TAG, VERBOSE) { "write($mode->ADB): $path" }
-                    // We need to keep the resource alive until the caller is done with the Sink object
+                    log(TAG, VERBOSE) { "file($mode->ADB, RW=$readWrite): $path" }
+                    // We need to keep the resource alive until the caller is done with the object
                     val resource = shizukuManager.serviceClient.get()
-                    adbOps { it.writeFile(path).callbacks { resource.close() } }
+                    adbOps {
+                        it.file(path, readWrite).callbacks {
+                            resource.close()
+                            log(TAG, VERBOSE) { "file($mode->ADB, RW=$readWrite): Closing resource for $path" }
+                        }
+                    }
                 }
 
                 else -> throw IOException("No matching mode available.")
             }
         } catch (e: IOException) {
-            log(TAG, WARN) { "write(path=$path, mode=$mode) failed." }
-            throw WriteException(path = path, cause = e)
+            log(TAG, WARN) { "file(path=$path, mode=$mode, RW=$readWrite) failed." }
+            if (readWrite) throw WriteException(path = path, cause = e)
+            else throw ReadException(path = path, cause = e)
         }
     }
 

diff --git a/app-common-io/src/main/java/eu/darken/sdmse/common/files/local/ipc/FileOpsClient.kt b/app-common-io/src/main/java/eu/darken/sdmse/common/files/local/ipc/FileOpsClient.kt
@@ -14,11 +14,9 @@ import eu.darken.sdmse.common.files.local.LocalPath
 import eu.darken.sdmse.common.files.local.LocalPathLookup
 import eu.darken.sdmse.common.files.local.LocalPathLookupExtended
 import eu.darken.sdmse.common.ipc.IpcClientModule
-import eu.darken.sdmse.common.ipc.sink
-import eu.darken.sdmse.common.ipc.source
+import eu.darken.sdmse.common.ipc.fileHandle
 import kotlinx.coroutines.flow.Flow
-import okio.Sink
-import okio.Source
+import okio.FileHandle
 import java.time.Instant
 
 class FileOpsClient @AssistedInject constructor(
@@ -95,14 +93,8 @@ class FileOpsClient @AssistedInject constructor(
         throw e.unwrapPropagation()
     }
 
-    fun readFile(path: LocalPath): Source = try {
-        fileOpsConnection.readFile(path).source()
-    } catch (e: Exception) {
-        throw e.unwrapPropagation()
-    }
-
-    fun writeFile(path: LocalPath): Sink = try {
-        fileOpsConnection.writeFile(path).sink()
+    fun file(path: LocalPath, readWrite: Boolean): FileHandle = try {
+        fileOpsConnection.file(path, readWrite).fileHandle(readWrite)
     } catch (e: Exception) {
         throw e.unwrapPropagation()
     }

diff --git a/app-common-io/src/main/java/eu/darken/sdmse/common/files/local/ipc/FileOpsHost.kt b/app-common-io/src/main/java/eu/darken/sdmse/common/files/local/ipc/FileOpsHost.kt
@@ -24,19 +24,20 @@ import eu.darken.sdmse.common.files.local.setOwnership
 import eu.darken.sdmse.common.files.local.setPermissions
 import eu.darken.sdmse.common.funnel.IPCFunnel
 import eu.darken.sdmse.common.ipc.IpcHostModule
+import eu.darken.sdmse.common.ipc.RemoteFileHandle
 import eu.darken.sdmse.common.ipc.RemoteInputStream
-import eu.darken.sdmse.common.ipc.RemoteOutputStream
-import eu.darken.sdmse.common.ipc.remoteInputStream
-import eu.darken.sdmse.common.ipc.toRemoteOutputStream
+import eu.darken.sdmse.common.ipc.fileHandle
+import eu.darken.sdmse.common.ipc.remoteFileHandle
 import eu.darken.sdmse.common.pkgs.pkgops.LibcoreTool
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.plus
 import kotlinx.coroutines.runBlocking
-import java.io.FileInputStream
-import java.io.FileOutputStream
 import java.io.IOException
 import javax.inject.Inject
 
+/**
+ * ROOT-side
+ */
 class FileOpsHost @Inject constructor(
     @AppScope private val appScope: CoroutineScope,
     private val dispatcherProvider: DispatcherProvider,
@@ -138,19 +139,11 @@ class FileOpsHost @Inject constructor(
         throw e.wrapToPropagate()
     }
 
-    override fun readFile(path: LocalPath): RemoteInputStream = try {
-        if (Bugs.isTrace) log(TAG, VERBOSE) { "readFile($path)..." }
-        FileInputStream(path.asFile()).remoteInputStream()
+    override fun file(path: LocalPath, readWrite: Boolean): RemoteFileHandle = try {
+        if (Bugs.isTrace) log(TAG, VERBOSE) { "file($path, $readWrite)..." }
+        path.asFile().fileHandle(readWrite).remoteFileHandle()
     } catch (e: Exception) {
-        log(TAG, ERROR) { "readFile(path=$path) failed\n${e.asLog()}" }
-        throw e.wrapToPropagate()
-    }
-
-    override fun writeFile(path: LocalPath): RemoteOutputStream = try {
-        if (Bugs.isTrace) log(TAG, VERBOSE) { "writeFile($path)..." }
-        FileOutputStream(path.asFile()).toRemoteOutputStream()
-    } catch (e: Exception) {
-        log(TAG, ERROR) { "writeFile(path=$path) failed\n${e.asLog()}" }
+        log(TAG, ERROR) { "file(path=$path) failed\n${e.asLog()}" }
         throw e.wrapToPropagate()
     }
 

diff --git a/app-common-io/src/main/java/eu/darken/sdmse/common/files/saf/DocumentFileExtensions.kt b/app-common-io/src/main/java/eu/darken/sdmse/common/files/saf/DocumentFileExtensions.kt
@@ -16,8 +16,8 @@ import java.io.IOException
 import java.util.Date
 
 
-enum class FileMode constructor(val value: String) {
-    WRITE("w"), READ("r")
+enum class FileMode(val value: String) {
+    READ_WRITE("rw"), WRITE("w"), READ("r")
 }
 
 internal fun DocumentFile.fstat(contentResolver: ContentResolver): StructStat? {