From c868aac2cc2dfb7aa0cce7d6a0422e5b1cbc91c6 Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Thu, 17 Sep 2026 03:30:30 +0700 Subject: [PATCH] perf(android): batch SAF lookups during download publication --- .../app/src/androidTest/AndroidManifest.xml | 19 ++ .../spotiflac/LookupTestControlProvider.java | 54 ++++ .../LookupTestDocumentsProvider.java | 232 ++++++++++++++++++ .../zarz/spotiflac/SafDocumentLookupTest.kt | 170 +++++++++++++ .../kotlin/com/zarz/spotiflac/MainActivity.kt | 7 +- .../com/zarz/spotiflac/MainActivitySafIo.kt | 1 + .../spotiflac/NativeFinalizerSafPublish.kt | 4 +- .../com/zarz/spotiflac/SafDocumentLookup.kt | 70 ++++++ .../com/zarz/spotiflac/SafDownloadHandler.kt | 142 +++++++---- .../download_queue_provider_finalization.dart | 10 + .../download_queue_provider_single_item.dart | 7 +- 11 files changed, 668 insertions(+), 48 deletions(-) create mode 100644 android/app/src/androidTest/AndroidManifest.xml create mode 100644 android/app/src/androidTest/java/com/zarz/spotiflac/LookupTestControlProvider.java create mode 100644 android/app/src/androidTest/java/com/zarz/spotiflac/LookupTestDocumentsProvider.java create mode 100644 android/app/src/androidTest/kotlin/com/zarz/spotiflac/SafDocumentLookupTest.kt create mode 100644 android/app/src/main/kotlin/com/zarz/spotiflac/SafDocumentLookup.kt diff --git a/android/app/src/androidTest/AndroidManifest.xml b/android/app/src/androidTest/AndroidManifest.xml new file mode 100644 index 00000000..d4aec7ec --- /dev/null +++ b/android/app/src/androidTest/AndroidManifest.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + diff --git a/android/app/src/androidTest/java/com/zarz/spotiflac/LookupTestControlProvider.java b/android/app/src/androidTest/java/com/zarz/spotiflac/LookupTestControlProvider.java new file mode 100644 index 00000000..a62ae876 --- /dev/null +++ b/android/app/src/androidTest/java/com/zarz/spotiflac/LookupTestControlProvider.java @@ -0,0 +1,54 @@ +package com.zarz.spotiflac; + +import android.content.ContentProvider; +import android.content.ContentValues; +import android.database.Cursor; +import android.net.Uri; +import android.os.Binder; +import android.os.Bundle; + +/** Issues fixture setup calls from the test APK's UID, never the app UID. */ +public final class LookupTestControlProvider extends ContentProvider { + @Override + public boolean onCreate() { + return true; + } + + @Override + public Bundle call(String method, String arg, Bundle extras) { + if (!method.startsWith("lookup.")) throw new IllegalArgumentException(method); + long identity = Binder.clearCallingIdentity(); + try { + return getContext().getContentResolver().call( + Uri.parse("content://com.spotiflac.test.documents.lookup"), method, arg, extras + ); + } finally { + Binder.restoreCallingIdentity(identity); + } + } + + @Override + public Cursor query(Uri uri, String[] projection, String selection, String[] args, String order) { + return null; + } + + @Override + public String getType(Uri uri) { + return null; + } + + @Override + public Uri insert(Uri uri, ContentValues values) { + return null; + } + + @Override + public int delete(Uri uri, String selection, String[] args) { + return 0; + } + + @Override + public int update(Uri uri, ContentValues values, String selection, String[] args) { + return 0; + } +} diff --git a/android/app/src/androidTest/java/com/zarz/spotiflac/LookupTestDocumentsProvider.java b/android/app/src/androidTest/java/com/zarz/spotiflac/LookupTestDocumentsProvider.java new file mode 100644 index 00000000..5c80f65c --- /dev/null +++ b/android/app/src/androidTest/java/com/zarz/spotiflac/LookupTestDocumentsProvider.java @@ -0,0 +1,232 @@ +package com.zarz.spotiflac; + +import android.content.Intent; +import android.database.Cursor; +import android.database.MatrixCursor; +import android.os.Binder; +import android.os.Bundle; +import android.os.CancellationSignal; +import android.os.ParcelFileDescriptor; +import android.provider.DocumentsContract; +import android.provider.DocumentsContract.Document; +import android.provider.DocumentsContract.Root; +import android.provider.DocumentsProvider; +import java.io.File; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.IOException; +import java.util.Arrays; +import java.util.LinkedHashMap; +import java.util.Map; + +/** Uses Java only: a standalone test APK provider cannot use the app's Kotlin runtime. */ +public final class LookupTestDocumentsProvider extends DocumentsProvider { + private static final class Entry { + final String id; + final String parent; + final String name; + final String mime; + final File file; + + Entry(String id, String parent, String name, String mime, File file) { + this.id = id; + this.parent = parent; + this.name = name; + this.mime = mime; + this.file = file; + } + } + + private final Map entries = new LinkedHashMap<>(); + private int sequence; + private int childQueries; + private int documentQueries; + private int closedCursors; + private String projectionMode = "normal"; + private boolean failFinalRename; + + private File fixtureDir() { + return new File(getContext().getCacheDir(), "lookup-provider"); + } + + @Override + public boolean onCreate() { + return true; + } + + @Override + public Bundle call(String method, String arg, Bundle extras) { + switch (method) { + case "lookup.reset": + File[] previous = fixtureDir().listFiles(); + if (previous != null) for (File file : previous) file.delete(); + fixtureDir().mkdirs(); + entries.clear(); + sequence = 0; + projectionMode = extras.getString("mode", "normal"); + failFinalRename = extras.getBoolean("failFinalRename"); + add("root", null, "Root", Document.MIME_TYPE_DIR); + for (int i = 0; i < extras.getInt("count"); i++) { + add("child:" + i, "root", "Track " + i + ".flac", "audio/flac"); + } + add("nested:音楽/opaque", "root", "音楽 🎵", Document.MIME_TYPE_DIR); + add("song:opaque/%", "nested:音楽/opaque", "歌 🎵.flac", "audio/flac"); + Entry original = add("original", "root", "Song.flac", "audio/flac"); + try (FileOutputStream output = new FileOutputStream(original.file)) { + output.write(new byte[] {'o', 'r', 'i', 'g', 'i', 'n', 'a', 'l'}); + } catch (IOException e) { + throw new IllegalStateException(e); + } + // Simulate the narrow tree grant normally issued by the picker. + long identity = Binder.clearCallingIdentity(); + try { + getContext().grantUriPermission( + extras.getString("targetPackage"), + DocumentsContract.buildTreeDocumentUri("com.spotiflac.test.documents.lookup", "root"), + Intent.FLAG_GRANT_READ_URI_PERMISSION | Intent.FLAG_GRANT_WRITE_URI_PERMISSION | + Intent.FLAG_GRANT_PREFIX_URI_PERMISSION + ); + } finally { + Binder.restoreCallingIdentity(identity); + } + resetCounters(); + return new Bundle(); + case "lookup.clearCounters": + resetCounters(); + return new Bundle(); + case "lookup.stats": + Bundle stats = new Bundle(); + stats.putInt("children", childQueries); + stats.putInt("documents", documentQueries); + stats.putInt("closed", closedCursors); + return stats; + default: + return super.call(method, arg, extras); + } + } + + private void resetCounters() { + childQueries = 0; + documentQueries = 0; + closedCursors = 0; + } + + private Entry add(String id, String parent, String name, String mime) { + Entry entry = new Entry(id, parent, name, mime, new File(fixtureDir(), "data-" + sequence++)); + entries.put(id, entry); + return entry; + } + + private MatrixCursor cursor(String[] columns) { + return new MatrixCursor(columns) { + @Override + public void close() { + if (!isClosed()) closedCursors++; + super.close(); + } + }; + } + + private void addEntry(MatrixCursor cursor, Entry entry) { + String[] columns = cursor.getColumnNames(); + Object[] row = new Object[columns.length]; + for (int i = 0; i < columns.length; i++) { + switch (columns[i]) { + case Document.COLUMN_DOCUMENT_ID: row[i] = entry.id; break; + case Document.COLUMN_DISPLAY_NAME: row[i] = entry.name; break; + case Document.COLUMN_MIME_TYPE: row[i] = entry.mime; break; + case Document.COLUMN_SIZE: row[i] = entry.file.length(); break; + case Document.COLUMN_FLAGS: + row[i] = Document.FLAG_SUPPORTS_WRITE | Document.FLAG_SUPPORTS_DELETE | + Document.FLAG_SUPPORTS_RENAME | Document.FLAG_DIR_SUPPORTS_CREATE; + break; + default: break; + } + } + cursor.addRow(row); + } + + @Override + public Cursor queryRoots(String[] projection) { + String[] columns = projection != null ? projection : + new String[] {Root.COLUMN_ROOT_ID, Root.COLUMN_DOCUMENT_ID, Root.COLUMN_TITLE}; + MatrixCursor cursor = new MatrixCursor(columns); + Object[] row = new Object[columns.length]; + for (int i = 0; i < columns.length; i++) { + if (Root.COLUMN_ROOT_ID.equals(columns[i]) || Root.COLUMN_DOCUMENT_ID.equals(columns[i])) row[i] = "root"; + else if (Root.COLUMN_TITLE.equals(columns[i])) row[i] = "Lookup fixture"; + } + cursor.addRow(row); + return cursor; + } + + @Override + public Cursor queryDocument(String documentId, String[] projection) throws FileNotFoundException { + documentQueries++; + Entry entry = entries.get(documentId); + if (entry == null) throw new FileNotFoundException(documentId); + MatrixCursor cursor = cursor(projection != null ? projection : defaultColumns()); + addEntry(cursor, entry); + return cursor; + } + + @Override + public Cursor queryChildDocuments(String parentId, String[] projection, String sortOrder) { + childQueries++; + boolean projected = projection != null && Arrays.asList(projection).contains(Document.COLUMN_DISPLAY_NAME); + if (projected && projectionMode.equals("throw")) throw new UnsupportedOperationException("projection"); + if (projected && projectionMode.equals("null")) return null; + String[] columns = projected && projectionMode.equals("missing") ? + new String[] {Document.COLUMN_DOCUMENT_ID} : projection != null ? projection : defaultColumns(); + MatrixCursor cursor = cursor(columns); + for (Entry entry : entries.values()) if (parentId.equals(entry.parent)) addEntry(cursor, entry); + return cursor; + } + + @Override + public boolean isChildDocument(String parentId, String documentId) { + Entry entry = entries.get(documentId); + while (entry != null && entry.parent != null) { + if (parentId.equals(entry.parent)) return true; + entry = entries.get(entry.parent); + } + return false; + } + + @Override + public String createDocument(String parentId, String mimeType, String displayName) { + String id = "created:" + sequence++; + add(id, parentId, displayName, mimeType); + return id; + } + + @Override + public String renameDocument(String documentId, String displayName) throws FileNotFoundException { + Entry entry = entries.get(documentId); + if (entry == null) throw new FileNotFoundException(documentId); + if (failFinalRename && entry.name.endsWith(".partial") && displayName.equals("Song.flac")) { + throw new FileNotFoundException("Injected publish rename failure"); + } + String newId = "renamed:" + sequence++; + entries.remove(documentId); + entries.put(newId, new Entry(newId, entry.parent, displayName, entry.mime, entry.file)); + return newId; + } + + @Override + public void deleteDocument(String documentId) { + Entry entry = entries.remove(documentId); + if (entry != null) entry.file.delete(); + } + + @Override + public ParcelFileDescriptor openDocument(String documentId, String mode, CancellationSignal signal) throws FileNotFoundException { + Entry entry = entries.get(documentId); + if (entry == null) throw new FileNotFoundException(documentId); + return ParcelFileDescriptor.open(entry.file, ParcelFileDescriptor.parseMode(mode)); + } + + private String[] defaultColumns() { + return new String[] {Document.COLUMN_DOCUMENT_ID, Document.COLUMN_DISPLAY_NAME, Document.COLUMN_MIME_TYPE, Document.COLUMN_SIZE}; + } +} diff --git a/android/app/src/androidTest/kotlin/com/zarz/spotiflac/SafDocumentLookupTest.kt b/android/app/src/androidTest/kotlin/com/zarz/spotiflac/SafDocumentLookupTest.kt new file mode 100644 index 00000000..f96f7346 --- /dev/null +++ b/android/app/src/androidTest/kotlin/com/zarz/spotiflac/SafDocumentLookupTest.kt @@ -0,0 +1,170 @@ +package com.zarz.spotiflac + +import android.net.Uri +import android.os.Bundle +import android.provider.DocumentsContract +import androidx.documentfile.provider.DocumentFile +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import java.io.File +import org.junit.Assert.assertArrayEquals +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class SafDocumentLookupTest { + private val context get() = InstrumentationRegistry.getInstrumentation().targetContext + private val authority = "com.spotiflac.test.documents.lookup" + private val controlUri = Uri.parse("content://$authority.control") + private val treeUri = DocumentsContract.buildTreeDocumentUri(authority, "root") + + private fun reset(count: Int = 0, mode: String = "normal", failFinalRename: Boolean = false): DocumentFile { + context.contentResolver.call(controlUri, "lookup.reset", null, Bundle().apply { + putInt("count", count) + putString("mode", mode) + putBoolean("failFinalRename", failFinalRename) + putString("targetPackage", context.packageName) + }) + return requireNotNull(DocumentFile.fromTreeUri(context, treeUri)) + } + + private fun stats() = requireNotNull(context.contentResolver.call(controlUri, "lookup.stats", null, null)) + + private fun clearCounters() { + context.contentResolver.call(controlUri, "lookup.clearCounters", null, null) + } + + @Test + fun projectedBatchReplacesThousandsOfNameQueriesAndClosesItsCursor() { + val parent = reset(count = 2000) + assertNull(parent.findFile("missing.flac")) + assertEquals(1, stats().getInt("children")) + assertEquals(2002, stats().getInt("documents")) + clearCounters() + + val found = findSafChildren(context, parent, setOf("Track 0.flac", "Track 1999.flac", "missing.flac")) + assertEquals(setOf("Track 0.flac", "Track 1999.flac"), found.keys) + assertEquals(1, stats().getInt("children")) + assertEquals(0, stats().getInt("documents")) + assertEquals(1, stats().getInt("closed")) + clearCounters() + assertNull(findSafChild(context, parent, "missing.flac")) + assertEquals(1, stats().getInt("children")) + assertEquals(0, stats().getInt("documents")) + clearCounters() + assertTrue(findSafChildren(context, parent, emptySet()).isEmpty()) + assertEquals(0, stats().getInt("children")) + } + + @Test + fun nestedOpaqueIdsUnicodeAndRenameKeepTheChildDocument() { + val parent = reset() + val directory = requireNotNull(findSafChild(context, parent, "音楽 🎵")) + assertEquals("nested:音楽/opaque", DocumentsContract.getDocumentId(directory.uri)) + assertTrue(directory.isDirectory) + val child = requireNotNull(findSafChild(context, directory, "歌 🎵.flac")) + val oldUri = child.uri + assertTrue(child.renameTo("Renamed.flac")) + assertNotEquals(oldUri, child.uri) + assertEquals("Renamed.flac", child.name) + assertEquals(child.uri, findSafChild(context, directory, "Renamed.flac")?.uri) + assertNull(findSafChild(context, parent, "Renamed.flac")) + assertNotNull(directory.createDirectory("Created")) + } + + @Test + fun unsupportedProjectionNullCursorAndMissingColumnUseLegacyLookup() { + for (mode in listOf("throw", "null", "missing")) { + val parent = reset(count = 3, mode = mode) + val found = requireNotNull(findSafChild(context, parent, "Song.flac")) + assertEquals("original", DocumentsContract.getDocumentId(found.uri)) + assertEquals(2, stats().getInt("children")) + assertEquals(5, stats().getInt("documents")) + assertEquals(if (mode == "missing") 7 else 6, stats().getInt("closed")) + } + } + + @Test + fun rawDocumentFileRetainsLegacyBehavior() { + val directory = File(context.cacheDir, "lookup-raw").apply { mkdirs() } + try { + File(directory, "local.flac").writeText("audio") + val found = findSafChild(context, DocumentFile.fromFile(directory), "local.flac") + assertEquals("local.flac", found?.name) + } finally { + directory.deleteRecursively() + } + } + + @Test + fun malformedTreeUriLeavesSourceIntact() { + val source = File(context.cacheDir, "lookup-invalid.flac").apply { writeText("original audio") } + try { + assertNull(SafDownloadHandler.writeFileToSaf( + context, "content://$authority/invalid", "", "Song.flac", "audio/flac", source.path, + )) + assertEquals("original audio", source.readText()) + } finally { + source.delete() + } + } + + @Test + fun failedPublicationRenameRestoresOriginalWhenProviderChangesDocumentIds() { + val parent = reset(failFinalRename = true) + val source = File(context.cacheDir, "lookup-source.flac").apply { writeText("replacement") } + try { + assertNull(SafDownloadHandler.writeFileToSaf(context, treeUri.toString(), "", "Song.flac", "audio/flac", source.path)) + val restored = requireNotNull(findSafChild(context, parent, "Song.flac")) + val content = context.contentResolver.openInputStream(restored.uri)?.bufferedReader()?.use { it.readText() } + assertEquals("original", content) + assertEquals("replacement", source.readText()) + assertNull(findSafChild(context, parent, "Song.flac.replaced")) + assertNull(findSafChild(context, parent, "Song.flac.partial")) + } finally { + source.delete() + } + } + + @Test + fun publicationPreservesBytesReportsStagesAndDetectsLaterExistingFile() { + val parent = reset(count = 2000) + val bytes = ByteArray(262144) { (it % 251).toByte() } + val source = File(context.cacheDir, "lookup-publish.flac").apply { writeBytes(bytes) } + try { + assertNull(findSafChild(context, parent, "Fresh.flac")) + clearCounters() + val result = requireNotNull(SafDownloadHandler.writeFileToSafIfAbsent( + context, treeUri.toString(), "", "Fresh.flac", "audio/flac", source.path, + )) + assertTrue(!result.alreadyExists) + assertEquals(4, stats().getInt("children")) + assertEquals(1, stats().getInt("documents")) + val expectedStages = setOf( + "lock_wait", "directory", "existing_check", "cleanup", "create", + "open", "copy", "sync", "close", "replace", "total", + ) + assertTrue(result.publishTimingsMs.keys.containsAll(expectedStages)) + assertTrue(result.publishTimingsMs.values.all { it >= 0 }) + val actual = context.contentResolver.openInputStream(Uri.parse(result.uri))?.use { it.readBytes() } + assertArrayEquals(bytes, actual) + assertEquals(result.uri, findSafChild(context, parent, "Fresh.flac")?.uri.toString()) + assertNull(findSafChild(context, parent, "Fresh.flac.partial")) + + source.writeText("must not replace existing audio") + val existing = requireNotNull(SafDownloadHandler.writeFileToSafIfAbsent( + context, treeUri.toString(), "", "Fresh.flac", "audio/flac", source.path, + )) + assertTrue(existing.alreadyExists) + assertEquals(result.uri, existing.uri) + assertArrayEquals(bytes, context.contentResolver.openInputStream(Uri.parse(existing.uri))?.use { it.readBytes() }) + } finally { + source.delete() + } + } +} diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt index 313b206b..8cbdbd28 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt @@ -1234,9 +1234,9 @@ class MainActivity: FlutterFragmentActivity() { if (treeUriStr.isBlank()) return@withContext null if (fileName.isBlank()) return@withContext null val dir = SafDownloadHandler.ensureDocumentDir(this@MainActivity, Uri.parse(treeUriStr), relativeDir) ?: return@withContext null - val existing = dir.findFile(fileName) + val existing = findSafChild(this@MainActivity, dir, fileName) val createdNew = existing == null - val doc = SafDownloadHandler.createOrReuseDocumentFile(dir, mimeType, fileName) + val doc = SafDownloadHandler.createOrReuseDocumentFile(this@MainActivity, dir, mimeType, fileName) ?: return@withContext null if (!writeUriFromPath(doc.uri, srcPath)) { if (createdNew) { @@ -1268,6 +1268,7 @@ class MainActivity: FlutterFragmentActivity() { .put("uri", writeResult.uri) .put("file_name", writeResult.fileName) .put("already_exists", writeResult.alreadyExists) + .put("publish_timings_ms", JSONObject(writeResult.publishTimingsMs)) .toString() } } @@ -1294,6 +1295,7 @@ class MainActivity: FlutterFragmentActivity() { JSONObject() .put("uri", writeResult.uri) .put("file_name", writeResult.fileName) + .put("publish_timings_ms", JSONObject(writeResult.publishTimingsMs)) .toString() } } @@ -1326,6 +1328,7 @@ class MainActivity: FlutterFragmentActivity() { JSONObject() .put("uri", writeResult.uri) .put("file_name", writeResult.fileName) + .put("publish_timings_ms", JSONObject(writeResult.publishTimingsMs)) .toString() } } diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivitySafIo.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivitySafIo.kt index b73ff607..ddf92963 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivitySafIo.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivitySafIo.kt @@ -367,6 +367,7 @@ internal fun MainActivity.writeSafSidecarLrc(audioUri: Uri, lrcContent: String): val lrcName = "$baseName.lrc" val target = SafDownloadHandler.createOrReuseDocumentFile( + this, parent, "application/octet-stream", lrcName diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerSafPublish.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerSafPublish.kt index 7a5ebc6b..8721528b 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerSafPublish.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerSafPublish.kt @@ -128,11 +128,13 @@ internal fun NativeDownloadFinalizer.publishDeferredSafOutput( srcPath = outputFile.absolutePath, )?.let { result -> alreadyExists = result.alreadyExists - SafDownloadHandler.UniqueWriteResult(result.uri, result.fileName) + SafDownloadHandler.UniqueWriteResult(result.uri, result.fileName, result.publishTimingsMs) } } ?: throw IllegalStateException("failed to publish deferred SAF output") val newUri = published.uri val publishedName = published.fileName + input.result.put("publish_timings_ms", JSONObject(published.publishTimingsMs)) + Log.d(TAG, "SAF publish timings (ms): ${published.publishTimingsMs}") outputFile.delete() state.filePath = newUri diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/SafDocumentLookup.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/SafDocumentLookup.kt new file mode 100644 index 00000000..0ddd4712 --- /dev/null +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/SafDocumentLookup.kt @@ -0,0 +1,70 @@ +package com.zarz.spotiflac + +import android.content.Context +import android.provider.DocumentsContract +import androidx.documentfile.provider.DocumentFile + +internal fun findSafChild( + context: Context, + parent: DocumentFile, + name: String, +): DocumentFile? = findSafChildren(context, parent, setOf(name))[name] + +/** Reads names with IDs in one query instead of querying each child's name. */ +internal fun findSafChildren( + context: Context, + parent: DocumentFile, + names: Set, +): Map { + if (names.isEmpty()) return emptyMap() + val children = try { + querySafChildren(context, parent, names) + } catch (_: Exception) { + null + } + // A successful miss is final. Retry only when the provider cannot support + // the projected query, keeping compatibility with unusual providers. + if (children != null) return children + return buildMap { + for (name in names) { + parent.findFile(name)?.let { put(name, it) } + } + } +} + +private fun querySafChildren( + context: Context, + parent: DocumentFile, + names: Set, +): Map? { + val parentUri = parent.uri + if (!DocumentsContract.isTreeUri(parentUri)) return null + val childrenUri = DocumentsContract.buildChildDocumentsUriUsingTree( + parentUri, + DocumentsContract.getDocumentId(parentUri), + ) + val projection = arrayOf( + DocumentsContract.Document.COLUMN_DOCUMENT_ID, + DocumentsContract.Document.COLUMN_DISPLAY_NAME, + ) + val cursor = context.contentResolver.query(childrenUri, projection, null, null, null) + ?: return null + return cursor.use { + val idColumn = it.getColumnIndex(DocumentsContract.Document.COLUMN_DOCUMENT_ID) + val nameColumn = it.getColumnIndex(DocumentsContract.Document.COLUMN_DISPLAY_NAME) + if (idColumn < 0 || nameColumn < 0) return null + val found = linkedMapOf() + while (it.moveToNext()) { + val name = it.getString(nameColumn) ?: continue + if (name !in names || name in found) continue + val id = it.getString(idColumn)?.takeIf(String::isNotEmpty) ?: return null + val childUri = DocumentsContract.buildDocumentUriUsingTree(parentUri, id) + // AndroidX 1.1.0 preserves the child ID in a tree document URI. + // fromSingleUri would disable directory operations and rename. + val child = DocumentFile.fromTreeUri(context, childUri) ?: return null + found[name] = child + if (found.size == names.size) break + } + found + } +} diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/SafDownloadHandler.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/SafDownloadHandler.kt index d0807800..a12d49cb 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/SafDownloadHandler.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/SafDownloadHandler.kt @@ -25,13 +25,33 @@ object SafDownloadHandler { // the exists check and reports already_exists. private val safNameLocks = KeyedLockPool() - data class UniqueWriteResult(val uri: String, val fileName: String) + data class UniqueWriteResult( + val uri: String, + val fileName: String, + val publishTimingsMs: Map = emptyMap(), + ) data class ExistingAwareWriteResult( val uri: String, val fileName: String, val alreadyExists: Boolean, + val publishTimingsMs: Map = emptyMap(), ) + private class PublishTrace { + private val started = System.nanoTime() + private var previous = started + private val stages = linkedMapOf() + + fun mark(stage: String) { + val now = System.nanoTime() + stages[stage] = (now - previous) / 1_000_000 + previous = now + } + + fun finish(): Map = stages.toMap() + + ("total" to (System.nanoTime() - started) / 1_000_000) + } + private fun withSafNameLock( treeUriStr: String, relativeDir: String, @@ -96,9 +116,9 @@ object SafDownloadHandler { val existingDir = findDocumentDir(context, treeUri, relativeDir) if (existingDir != null && req.optString("album_folder_template", "").isBlank()) { - val existing = existingDir.findFile(fileName) + val existing = findSafChild(context, existingDir, fileName) if (existing != null && existing.isFile && existing.length() > 0) { - deleteStaleStagedFiles(existingDir, fileName, outputExt) + deleteStaleStagedFiles(context, existingDir, fileName, outputExt) val obj = JSONObject() obj.put("success", true) obj.put("message", "File already exists") @@ -110,7 +130,7 @@ object SafDownloadHandler { } if (deferSafPublish) { - existingDir?.let { deleteStaleStagedFiles(it, fileName, outputExt) } + existingDir?.let { deleteStaleStagedFiles(context, it, fileName, outputExt) } val workingExt = outputExt.ifBlank { ".tmp" } val workingFile = backend.createTemporaryMediaFile(context, "native_saf_work_", workingExt) return try { @@ -158,8 +178,8 @@ object SafDownloadHandler { // creating the staged document: reusing it would let a shorter new // write leave the old tail bytes in place (fd truncation is // best-effort on some providers). - deleteStaleStagedFiles(targetDir, fileName, outputExt) - var document = createOrReuseDocumentFile(targetDir, stagedMimeType, stagedFileName) + deleteStaleStagedFiles(context, targetDir, fileName, outputExt) + var document = createOrReuseDocumentFile(context, targetDir, stagedMimeType, stagedFileName) ?: return errorJson("Failed to create SAF file") var pfd: android.os.ParcelFileDescriptor? = null @@ -224,6 +244,7 @@ object SafDownloadHandler { } val actualMimeType = mimeTypeForExt(actualExt) val replacement = createOrReuseDocumentFile( + context, targetDir, if (useStagedOutput) STAGED_SAF_MIME_TYPE else actualMimeType, actualStagedFileName @@ -260,7 +281,7 @@ object SafDownloadHandler { } else if (useStagedOutput) { // Legacy caller (foreground Dart queue): publish here by // renaming the staged file to its final name. - val published = replaceFinalDocument(targetDir, document, finalFileName) + val published = replaceFinalDocument(context, targetDir, document, finalFileName) if (published == null) { document.delete() return errorJson("Failed to publish SAF download") @@ -294,29 +315,32 @@ object SafDownloadHandler { * document, or null when the swap failed (the caller owns [document]). */ private fun replaceFinalDocument( + context: Context, targetDir: DocumentFile, document: DocumentFile, finalName: String ): DocumentFile? { - val existingFinal = targetDir.findFile(finalName) + val existingFinal = findSafChild(context, targetDir, finalName) var aside: DocumentFile? = null if (existingFinal != null && existingFinal.uri != document.uri) { val asideName = buildReplacedSafFileName(finalName) try { - targetDir.findFile(asideName)?.delete() + findSafChild(context, targetDir, asideName)?.delete() } catch (_: Exception) { } if (!existingFinal.renameTo(asideName)) { return null } - aside = targetDir.findFile(asideName) ?: existingFinal + // TreeDocumentFile.renameTo updates this object's URI, including + // providers whose document IDs change with the display name. + aside = existingFinal } if (!document.renameTo(finalName)) { aside?.renameTo(finalName) return null } aside?.delete() - return targetDir.findFile(finalName) ?: document + return document } private fun buildReplacedSafFileName(fileName: String): String { @@ -361,7 +385,14 @@ object SafDownloadHandler { ): String? { val finalName = sanitizeFilename(fileName) return withSafNameLock(treeUriStr, sanitizeRelativeDir(relativeDir), finalName) { - writeFileToSafLocked(context, treeUriStr, relativeDir, finalName, srcPath) + try { + val targetDir = ensureDocumentDir(context, Uri.parse(treeUriStr), relativeDir) + ?: return@withSafNameLock null + writeFileToSafLocked(context, targetDir, finalName, srcPath) + } catch (e: Exception) { + android.util.Log.w("SpotiFLAC", "Failed to write file to SAF: ${e.message}") + null + } } } @@ -376,22 +407,27 @@ object SafDownloadHandler { ): UniqueWriteResult? { val safeRelativeDir = sanitizeRelativeDir(relativeDir) val preferredName = sanitizeFilenamePreservingSuffix(fileName, preservedSuffix) + val trace = PublishTrace() return withSafNameLock(treeUriStr, safeRelativeDir, preferredName) { + trace.mark("lock_wait") val treeUri = Uri.parse(treeUriStr) val targetDir = ensureDocumentDir(context, treeUri, safeRelativeDir) ?: return@withSafNameLock null + trace.mark("directory") val availableName = findAvailableFileName( + context, targetDir, preferredName, preservedSuffix, ) + trace.mark("existing_check") val uri = writeFileToSafLocked( context, - treeUriStr, - safeRelativeDir, + targetDir, availableName, srcPath, + trace, ) ?: return@withSafNameLock null - UniqueWriteResult(uri = uri, fileName = availableName) + UniqueWriteResult(uri = uri, fileName = availableName, publishTimingsMs = trace.finish()) } } @@ -411,23 +447,27 @@ object SafDownloadHandler { variantFileName, preservedSuffix, ) + val trace = PublishTrace() return withSafNameLock(treeUriStr, safeRelativeDir, cleanName) { + trace.mark("lock_wait") val treeUri = Uri.parse(treeUriStr) val targetDir = ensureDocumentDir(context, treeUri, safeRelativeDir) ?: return@withSafNameLock null - val selectedName = if (targetDir.findFile(cleanName) == null) { + trace.mark("directory") + val selectedName = if (findSafChild(context, targetDir, cleanName) == null) { cleanName } else { - findAvailableFileName(targetDir, preferredVariant, preservedSuffix) + findAvailableFileName(context, targetDir, preferredVariant, preservedSuffix) } + trace.mark("existing_check") val uri = writeFileToSafLocked( context, - treeUriStr, - safeRelativeDir, + targetDir, selectedName, srcPath, + trace, ) ?: return@withSafNameLock null - UniqueWriteResult(uri = uri, fileName = selectedName) + UniqueWriteResult(uri = uri, fileName = selectedName, publishTimingsMs = trace.finish()) } } @@ -441,29 +481,35 @@ object SafDownloadHandler { ): ExistingAwareWriteResult? { val safeRelativeDir = sanitizeRelativeDir(relativeDir) val finalName = sanitizeFilename(fileName) + val trace = PublishTrace() return withSafNameLock(treeUriStr, safeRelativeDir, finalName) { + trace.mark("lock_wait") val treeUri = Uri.parse(treeUriStr) val targetDir = ensureDocumentDir(context, treeUri, safeRelativeDir) ?: return@withSafNameLock null - val existing = targetDir.findFile(finalName) + trace.mark("directory") + val existing = findSafChild(context, targetDir, finalName) + trace.mark("existing_check") if (existing != null && existing.isFile && existing.length() > 0L) { return@withSafNameLock ExistingAwareWriteResult( uri = existing.uri.toString(), fileName = existing.name ?: finalName, alreadyExists = true, + publishTimingsMs = trace.finish(), ) } val uri = writeFileToSafLocked( context, - treeUriStr, - safeRelativeDir, + targetDir, finalName, srcPath, + trace, ) ?: return@withSafNameLock null ExistingAwareWriteResult( uri = uri, fileName = finalName, alreadyExists = false, + publishTimingsMs = trace.finish(), ) } } @@ -490,18 +536,19 @@ object SafDownloadHandler { } private fun findAvailableFileName( + context: Context, parent: DocumentFile, preferredName: String, preservedSuffix: String, ): String { - if (parent.findFile(preferredName) == null) return preferredName + if (findSafChild(context, parent, preferredName) == null) return preferredName for (counter in 2..9999) { val candidate = appendFilenameCounter( preferredName, counter.toLong(), preservedSuffix, ) - if (parent.findFile(candidate) == null) return candidate + if (findSafChild(context, parent, candidate) == null) return candidate } return appendFilenameCounter( preferredName, @@ -540,20 +587,20 @@ object SafDownloadHandler { private fun writeFileToSafLocked( context: Context, - treeUriStr: String, - relativeDir: String, + targetDir: DocumentFile, finalName: String, - srcPath: String + srcPath: String, + trace: PublishTrace = PublishTrace(), ): String? { var stagedDocument: DocumentFile? = null return try { - val treeUri = Uri.parse(treeUriStr) - val targetDir = ensureDocumentDir(context, treeUri, relativeDir) ?: return null val ext = normalizeExt(finalName.substringAfterLast('.', "")) val stagedName = buildStagedSafFileName(finalName) - deleteStaleStagedFiles(targetDir, finalName, ext) - val document = createOrReuseDocumentFile(targetDir, STAGED_SAF_MIME_TYPE, stagedName) + deleteStaleStagedFiles(context, targetDir, finalName, ext) + trace.mark("cleanup") + val document = createOrReuseDocumentFile(context, targetDir, STAGED_SAF_MIME_TYPE, stagedName) ?: return null + trace.mark("create") stagedDocument = document val outputStream = context.contentResolver.openOutputStream(document.uri, "wt") if (outputStream == null) { @@ -561,14 +608,19 @@ object SafDownloadHandler { stagedDocument = null return null } + trace.mark("open") outputStream.use { output -> File(srcPath).inputStream().use { input -> - input.copyTo(output) + input.copyTo(output, bufferSize = 64 * 1024) } + trace.mark("copy") syncOutputStream(output) + trace.mark("sync") } + trace.mark("close") - val published = replaceFinalDocument(targetDir, document, finalName) + val published = replaceFinalDocument(context, targetDir, document, finalName) + trace.mark("replace") if (published == null) { document.delete() return null @@ -644,15 +696,20 @@ object SafDownloadHandler { return "$safeName.partial" } - private fun deleteStaleStagedFiles(parent: DocumentFile, fileName: String, outputExt: String) { + private fun deleteStaleStagedFiles(context: Context, parent: DocumentFile, fileName: String, outputExt: String) { val stagedNames = linkedSetOf( buildStagedSafFileName(fileName), buildLegacyStagedSafFileName(fileName, outputExt), buildReplacedSafFileName(fileName) ) - for (stagedName in stagedNames) { + val staleDocuments = try { + findSafChildren(context, parent, stagedNames) + } catch (_: Exception) { + return + } + for (document in staleDocuments.values) { try { - parent.findFile(stagedName)?.delete() + document.delete() } catch (_: Exception) { } } @@ -740,7 +797,7 @@ object SafDownloadHandler { var current = DocumentFile.fromTreeUri(context, treeUri) ?: return null val parts = safeRelativeDir.split("/").filter { it.isNotBlank() } for (part in parts) { - val existing = current.findFile(part) + val existing = findSafChild(context, current, part) current = if (existing != null && existing.isDirectory) { existing } else { @@ -748,7 +805,7 @@ object SafDownloadHandler { val createdName = created.name ?: part if (createdName != part) { created.delete() - current.findFile(part) ?: return null + findSafChild(context, current, part) ?: return null } else { created } @@ -769,7 +826,7 @@ object SafDownloadHandler { val parts = safeRelativeDir.split("/").filter { it.isNotBlank() } for (part in parts) { - val existing = current.findFile(part) + val existing = findSafChild(context, current, part) if (existing == null || !existing.isDirectory) return null current = existing } @@ -777,6 +834,7 @@ object SafDownloadHandler { } internal fun createOrReuseDocumentFile( + context: Context, parent: DocumentFile, mimeType: String, fileName: String @@ -785,7 +843,7 @@ object SafDownloadHandler { if (safeFileName.isBlank()) return null synchronized(safDirLock) { - val existing = parent.findFile(safeFileName) + val existing = findSafChild(context, parent, safeFileName) if (existing != null && existing.isFile) { return existing } @@ -796,7 +854,7 @@ object SafDownloadHandler { return created } - val winner = parent.findFile(safeFileName) + val winner = findSafChild(context, parent, safeFileName) if (winner != null && winner.isFile) { if (winner.uri != created.uri) { try { diff --git a/lib/providers/download_queue_provider_finalization.dart b/lib/providers/download_queue_provider_finalization.dart index 3b45b3ea..c1919700 100644 --- a/lib/providers/download_queue_provider_finalization.dart +++ b/lib/providers/download_queue_provider_finalization.dart @@ -447,6 +447,7 @@ extension _DownloadQueueFinalization on DownloadQueueNotifier { mimeType: mimeType, srcPath: srcPath, ); + _logSafPublishTimings(result); final uri = (result['uri'] as String? ?? '').trim(); final publishedName = (result['file_name'] as String? ?? '').trim(); if (uri.isEmpty || publishedName.isEmpty) return null; @@ -461,6 +462,13 @@ extension _DownloadQueueFinalization on DownloadQueueNotifier { } } + void _logSafPublishTimings(Map result) { + final timings = result['publish_timings_ms']; + if (timings is Map && timings.isNotEmpty) { + _log.d('SAF publish timings (ms): $timings'); + } + } + Future<({String uri, String fileName})?> _writeTempToSafUnique({ required String treeUri, required String relativeDir, @@ -478,6 +486,7 @@ extension _DownloadQueueFinalization on DownloadQueueNotifier { srcPath: srcPath, preservedSuffix: preservedSuffix, ); + _logSafPublishTimings(result); final uri = (result['uri'] as String? ?? '').trim(); final publishedName = (result['file_name'] as String? ?? '').trim(); if (uri.isEmpty || publishedName.isEmpty) return null; @@ -507,6 +516,7 @@ extension _DownloadQueueFinalization on DownloadQueueNotifier { srcPath: srcPath, preservedSuffix: preservedSuffix, ); + _logSafPublishTimings(result); final uri = (result['uri'] as String? ?? '').trim(); final publishedName = (result['file_name'] as String? ?? '').trim(); if (uri.isEmpty || publishedName.isEmpty) return null; diff --git a/lib/providers/download_queue_provider_single_item.dart b/lib/providers/download_queue_provider_single_item.dart index d12d58ae..bcc6ec54 100644 --- a/lib/providers/download_queue_provider_single_item.dart +++ b/lib/providers/download_queue_provider_single_item.dart @@ -1040,9 +1040,10 @@ class _DownloadRun { final localPath = filePath; if (localPath == null || isContentUri(localPath)) return true; final localFile = File(localPath); - if (!await localFile.exists() || await localFile.length() <= 0) { - return false; - } + if (!await localFile.exists()) return false; + final localSize = await localFile.length(); + if (localSize <= 0) return false; + _log.d('Publishing finalized SAF output ($localSize bytes)'); var finalName = normalizeOptionalString(finalSafFileName) ??