android: a picked file streams to disk instead of into the heap

Both Android import paths — a single picked file and a folder import —
read each file whole into a ByteArray and handed it across the FFI, where
Rust wrote it back out to a staging file because the importers take a
path. A large video or PDF was the whole file in the heap, twice.
Kotlin now streams the content into the app's cache, and
ImportBatch::import_file takes that path: StagedFile::adopt moves it
under the name dispatch needs (a rename on one filesystem, a copy and
delete otherwise) and removes it once the import is done.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

android/app/src/main/java/com/gregshuflin/synchronicity/BatchImport.kt

@@ -2,12 +2,16 @@ package com.gregshuflin.synchronicity
 
 import android.content.Context
 import android.net.Uri
+import android.util.Log
+import java.io.File
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.withContext
 import uniffi.synchronicity.ImportBatch
 import uniffi.synchronicity.ImportBatchSummary
 import uniffi.synchronicity.SyncException
 
+private const val TAG = "BatchImport"
+
 /**
  * Open a batch import: one tracked, cancelable job covering [totalFiles] files holding roughly
  * [totalBytes] bytes, fed one file at a time through the returned [BatchImport]. Both totals are
@@ -38,27 +42,56 @@ fun NativeLib.beginImportBatch(
  */
 suspend fun NativeLib.importPickedFile(context: Context, uri: Uri): Result<String> =
     withContext(Dispatchers.IO) {
-        val resolver = context.contentResolver
         val name = getFileNameFromUri(context, uri) ?: "import"
-        val bytes =
-            resolver.openInputStream(uri)?.use { it.readBytes() }
+        val copy =
+            copyToImportCache(context, uri)
                 ?: return@withContext Result.failure(
                     SyncException.InvalidInput("Could not read $name")
                 )
-        result {
-            val batch = beginImportBatch(uri.toString(), name, 1, bytes.size.toLong()).bind()
-            val imported =
-                batch.importFile(name, resolver.getType(uri), bytes, parentId = null).also {
-                    batch.finish()
-                }
-            imported.bind()
-                ?: return@withContext Result.failure(
-                    SyncException.InvalidInput("Unsupported file type: $name")
-                )
-            name
+        try {
+            result {
+                val batch = beginImportBatch(uri.toString(), name, 1, copy.length()).bind()
+                val imported =
+                    batch.importFile(name, context.contentResolver.getType(uri), copy, null).also {
+                        batch.finish()
+                    }
+                imported.bind()
+                    ?: return@withContext Result.failure(
+                        SyncException.InvalidInput("Unsupported file type: $name")
+                    )
+                name
+            }
+        } finally {
+            copy.delete()
         }
     }
 
+/**
+ * Stream [uri]'s content into a new file in the app's cache, for [BatchImport.importFile] to take
+ * over — rather than reading it into a `ByteArray`, which for a large video or PDF is the whole
+ * file in the heap. The caller deletes it afterwards; if the import got as far as moving it into
+ * Rust's staging, that delete finds nothing.
+ *
+ * Null, logged, when the content can't be opened or copied: a picker hands out URIs that fail to
+ * open for a dozen unrelated reasons, none of which should stop the rest of a folder import.
+ */
+@Suppress("TooGenericExceptionCaught")
+fun copyToImportCache(context: Context, uri: Uri): File? {
+    var copy: File? = null
+    return try {
+        context.contentResolver.openInputStream(uri)?.use { source ->
+            File.createTempFile("import-", null, context.cacheDir).also { file ->
+                copy = file
+                file.outputStream().use { source.copyTo(it) }
+            }
+        }
+    } catch (e: Exception) {
+        Log.i(TAG, "Could not read $uri", e)
+        copy?.delete()
+        null
+    }
+}
+
 /**
  * One batch import in progress — the Kotlin half of `ffi.rs`'s `ImportBatch`, wrapping each call in
  * [ffiCatching].
@@ -75,16 +108,17 @@ class BatchImport internal constructor(private val batch: ImportBatch) {
     fun isCanceled(): Boolean = batch.isCanceled()
 
     /**
-     * Import one file's bytes. Succeeds with the new document id, or with null when no supported
-     * type recognized the file (a skip, not a failure). [mimeType] is only a fallback for dispatch,
-     * used when [fileName] has no extension a supported type claims.
+     * Import [file], a copy from [copyToImportCache], under the name the user knows it by. Succeeds
+     * with the new document id, or with null when no supported type recognized the file (a skip,
+     * not a failure). [mimeType] is only a fallback for dispatch, used when [fileName] has no
+     * extension a supported type claims.
      */
     fun importFile(
         fileName: String,
         mimeType: String?,
-        bytes: ByteArray,
+        file: File,
         parentId: String?,
-    ): Result<String?> = ffiCatching { batch.importFile(fileName, mimeType, bytes, parentId) }
+    ): Result<String?> = ffiCatching { batch.importFile(fileName, mimeType, file.path, parentId) }
 
     /** Count a file the caller couldn't read, so the job's denominator still advances. */
     fun skipFile(fileName: String, byteSize: Long, reason: String) {

android/app/src/main/java/com/gregshuflin/synchronicity/documents/ui/DocumentDirectoryImport.kt

@@ -7,6 +7,7 @@ import android.util.Log
 import com.gregshuflin.synchronicity.BatchImport
 import com.gregshuflin.synchronicity.NativeLib
 import com.gregshuflin.synchronicity.beginImportBatch
+import com.gregshuflin.synchronicity.copyToImportCache
 import com.gregshuflin.synchronicity.ui.SharedCopy
 import com.gregshuflin.synchronicity.userMessage
 import kotlinx.coroutines.Dispatchers
@@ -212,42 +213,35 @@ private class DirectoryImporter(
     }
 
     /**
-     * Hand one file's bytes to the batch. Which importer runs, and whether the type is supported at
-     * all, is decided Rust-side from the name and MIME type — this side only reads the bytes.
+     * Hand one file to the batch. Which importer runs, and whether the type is supported at all, is
+     * decided Rust-side from the name and MIME type — this side only copies the content out.
      */
     private fun importFileEntry(entry: TreeEntry, parentNode: String?) {
-        // Asked before the bytes are read, not after: they cross the FFI as one ByteArray, and a
-        // video sitting in a folder of PDFs would otherwise be gigabytes pulled into the heap
-        // only to be skipped.
+        // Asked before the content is copied, not after: a video sitting in a folder of PDFs would
+        // otherwise be gigabytes written to the cache only to be skipped.
         if (!NativeLib.ffi { import().supports(entry.name, entry.mimeType) }.getOrDefault(false)) {
             batch.skipFile(entry.name, entry.size, "unsupported type (${entry.mimeType})")
             return
         }
         val uri = DocumentsContract.buildDocumentUriUsingTree(treeUri, entry.docId)
-        val bytes = readBytes(uri)
-        if (bytes == null) {
+        val copy = copyToImportCache(context, uri)
+        if (copy == null) {
             batch.skipFile(entry.name, entry.size, "could not read file")
             return
         }
-        batch
-            .importFile(entry.name, entry.mimeType, bytes, parentNode)
-            .onSuccess { docId ->
-                if (docId != null) Log.i(TAG, "Imported ${entry.name} as document $docId")
-            }
-            .onFailure { Log.i(TAG, "Skipping ${entry.name} (import failed: ${it.userMessage})") }
-    }
-
-    // Kept separate so the broad catch covers only the stream read: a directory picker hands out
-    // URIs that can fail to open for a dozen unrelated reasons, none of which should stop the
-    // rest of the batch.
-    @Suppress("TooGenericExceptionCaught")
-    private fun readBytes(uri: Uri): ByteArray? =
         try {
-            context.contentResolver.openInputStream(uri)?.use { it.readBytes() }
-        } catch (e: Exception) {
-            Log.i(TAG, "Could not read $uri", e)
-            null
+            batch
+                .importFile(entry.name, entry.mimeType, copy, parentNode)
+                .onSuccess { docId ->
+                    if (docId != null) Log.i(TAG, "Imported ${entry.name} as document $docId")
+                }
+                .onFailure {
+                    Log.i(TAG, "Skipping ${entry.name} (import failed: ${it.userMessage})")
+                }
+        } finally {
+            copy.delete()
         }
+    }
 
     /** Create a Collection node, returning its id, or null (logged) if creation failed. */
     private fun createCollection(parentNode: String?, name: String): String? =

rust/lib/src/ffi/import.rs

@@ -1,4 +1,4 @@
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::sync::{Arc, Mutex};
 
 use crate::copy::busy;
@@ -246,15 +246,22 @@ impl ImportBatch {
         state.handle.as_ref().is_some_and(JobHandle::is_canceled)
     }
 
-    /// Import one file's bytes into this batch, returning its new document id,
-    /// or `None` if no supported type recognized it (a skip, not a failure).
+    /// Import one file into this batch, returning its new document id, or
+    /// `None` if no supported type recognized it (a skip, not a failure).
+    ///
+    /// `path` is a copy the caller has already written out — on Android, a
+    /// content URI streamed into the app's cache — and the batch takes it over:
+    /// it is moved into staging (see [`StagedFile`]) and removed once the
+    /// import is done, whatever the outcome. A path rather than the bytes
+    /// themselves so that a large video or PDF never has to fit in the heap on
+    /// either side of the FFI. `file_name` is the name the user knows it by,
+    /// which the staged copy takes, since that is where its title comes from.
     ///
     /// Dispatch is [`crate::documents::import::import_file`]'s — the same table
     /// the desktop and CLI imports use — so this path gets every type they do,
     /// including the metadata each agent's importer extracts (a track's tags, a
     /// comic's pages) rather than the bare title-and-extension the one-file FFI
-    /// methods write. Reaching it costs staging the bytes as a real file first
-    /// (see [`StagedFile`]), because those importers take a path.
+    /// methods write.
     ///
     /// `mime_type` is a fallback for dispatch, not the primary key: it's used
     /// only when `file_name` has no extension a supported type claims.
@@ -262,11 +269,12 @@ impl ImportBatch {
         &self,
         file_name: String,
         mime_type: Option<String>,
-        bytes: Vec<u8>,
+        path: String,
         parent_id: Option<DocumentID>,
     ) -> Result<Option<DocumentID>, SyncError> {
-        let size = bytes.len() as u64;
-        let result = self.import_one(&file_name, mime_type.as_deref(), &bytes, parent_id);
+        let path = PathBuf::from(path);
+        let size = std::fs::metadata(&path).map(|m| m.len()).unwrap_or(0);
+        let result = self.import_one(&file_name, mime_type.as_deref(), &path, parent_id);
 
         let mut state = self.state.lock().unwrap_or_else(|e| e.into_inner());
         match &result {
@@ -322,13 +330,13 @@ impl ImportBatch {
         &self,
         file_name: &str,
         mime_type: Option<&str>,
-        bytes: &[u8],
+        path: &Path,
         parent: Option<DocumentID>,
     ) -> Result<Option<DocumentID>, SyncError> {
         let origin_node = self.core.local_node_pubkey_required()?;
         let store = self.core.store();
         let staged =
-            StagedFile::write(&store, &staged_name(file_name, mime_type), bytes).map_err(|e| {
+            StagedFile::adopt(&store, &staged_name(file_name, mime_type), path).map_err(|e| {
                 SyncError::Internal { msg: format!("could not stage {file_name}: {e}") }
             })?;
         let target = ImportTarget {
@@ -379,8 +387,8 @@ fn staged_name(file_name: &str, mime_type: Option<&str>) -> String {
     }
 }
 
-/// Bytes handed across the FFI, written out as a real file so the shared
-/// path-based importers can read them, and removed again when this is dropped —
+/// A file handed across the FFI, moved under the name the shared path-based
+/// importers dispatch and title it by, and removed again when this is dropped —
 /// on the error paths too, since an import that failed half way still leaves the
 /// staged copy behind.
 ///
@@ -429,12 +437,21 @@ fn sweep_stale_staging(store: &Store) {
 }
 
 impl StagedFile {
-    fn write(store: &Store, name: &str, bytes: &[u8]) -> std::io::Result<Self> {
+    /// Take over `src` as `name` in a fresh staging directory: a rename where
+    /// the two share a filesystem (the app's cache and its store do, on
+    /// Android), and a copy and delete where they don't.
+    fn adopt(store: &Store, name: &str, src: &Path) -> std::io::Result<Self> {
         let dir = store.path.join(STAGING_DIR).join(uuid::Uuid::new_v4().to_string());
         std::fs::create_dir_all(&dir)?;
-        let path = dir.join(name);
-        std::fs::write(&path, bytes)?;
-        Ok(StagedFile { dir, path })
+        // Built before the move so that a failed copy still removes the directory.
+        let staged = StagedFile { path: dir.join(name), dir };
+        if std::fs::rename(src, &staged.path).is_err() {
+            std::fs::copy(src, &staged.path)?;
+            if let Err(e) = std::fs::remove_file(src) {
+                log::warn!("import batch: could not remove {}: {e}", src.display());
+            }
+        }
+        Ok(staged)
     }
 }
 
@@ -449,8 +466,26 @@ impl Drop for StagedFile {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::test_support::{TestStore, store};
     use rstest::rstest;
 
+    /// The caller's copy moves under the name dispatch needs, and is gone —
+    /// directory and all — once the import lets go of it.
+    #[rstest]
+    fn a_staged_file_takes_over_its_source_and_cleans_up(store: TestStore) {
+        let src = store.path.join("picked");
+        std::fs::write(&src, b"bytes").unwrap();
+
+        let staged = StagedFile::adopt(&store, "Chapter 1.cbz", &src).unwrap();
+        assert!(!src.exists());
+        assert!(staged.path.ends_with("Chapter 1.cbz"));
+        assert_eq!(std::fs::read(&staged.path).unwrap(), b"bytes");
+
+        let dir = staged.dir.clone();
+        drop(staged);
+        assert!(!dir.exists());
+    }
+
     /// A display name from a picker is not a path, and is about to become one.
     #[rstest]
     // Already dispatchable: the name wins, and its stem becomes the title.