Code reviews: apply review cleanups to the media-save fix and its tests

- Extract the duplicated drive-the-save harness (temp file, runBlocking save,
  success/error assertions) from both instrumented e2e tests into
  MediaSaverTestSupport, following the AvifInstrumentedTestSupport precedent,
  with UUID-based filenames per the existing convention.
- MediaSaverToDiskMediaStoreTest: record inserted rows as item Uris
  (ContentUris.withAppendedId) instead of Pair + hand-built _ID selection; trim
  the KDoc paragraph that re-quoted the MediaProvider rejection verbatim - the
  canonical copy lives on MediaStoreTarget.
- MediaSaverToDiskLegacyStorageTest: derive watchedDirs from
  MediaStoreTarget.entries instead of a third hand-maintained directory list;
  move the exact run recipe (assemble, install -g, am instrument) into the class
  KDoc and point the skip message at it; unfold the write-probe .also puzzle.
- MediaSaverToDisk: drop the outer withContext in saveDownloadingIfNeeded (both
  delegates now dispatch themselves, leaving the decision in the leaf writers);
  scope `val extension` to the pre-Q branch that uses it; drop the rot-prone
  composable file name from save()'s KDoc.

Considered and left alone: isSaveableMimeType deriving from MediaStoreTarget.of
(kept - one definition of the accepted set beats re-spelling the prefix triple);
the nested Dispatchers.IO in save()/downloadAndSave (load-bearing for direct
call sites, fast-path no-op when nested); the redundant launch(Dispatchers.IO)
at two call sites outside this branch.

amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/actions/MediaSaverTestSupport.kt

@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2025 Vitor Pamplona
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
+ * Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
+ * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+package com.vitorpamplona.amethyst.ui.actions
+
+import android.content.Context
+import kotlinx.coroutines.runBlocking
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertTrue
+import java.io.File
+import java.util.UUID
+
+/**
+ * Shared harness for the MediaSaverToDisk instrumented tests: writes a small payload
+ * file, drives [MediaSaverToDisk.save] with the given MIME type, and asserts the save
+ * reported success. Package-level support object per the AvifInstrumentedTestSupport
+ * precedent.
+ */
+object MediaSaverTestSupport {
+    /** Drives one save and fails the test if it reported an error or never succeeded. */
+    fun saveAndAssertSuccess(
+        context: Context,
+        mimeType: String,
+    ) {
+        val localFile = File(context.cacheDir, "media-saver-${UUID.randomUUID()}.bin")
+        localFile.writeBytes(ByteArray(2048) { it.toByte() })
+
+        var failure: Throwable? = null
+        var succeeded = false
+
+        try {
+            runBlocking {
+                MediaSaverToDisk.save(
+                    localFile = localFile,
+                    mimeType = mimeType,
+                    context = context,
+                    onSuccess = { succeeded = true },
+                    onError = { failure = it },
+                )
+            }
+        } finally {
+            localFile.delete()
+        }
+
+        // Surfaces e.g. the #4009 IllegalArgumentException as the test failure message.
+        assertNull("save() reported an error: ${failure?.message}", failure)
+        assertTrue("save() never reported success", succeeded)
+    }
+}

amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/actions/MediaSaverToDiskLegacyStorageTest.kt

@@ -27,11 +27,8 @@ import android.os.Environment
 import android.os.ParcelFileDescriptor
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.platform.app.InstrumentationRegistry
-import kotlinx.coroutines.runBlocking
 import org.junit.After
 import org.junit.Assert.assertEquals
-import org.junit.Assert.assertNull
-import org.junit.Assert.assertTrue
 import org.junit.Assume.assumeTrue
 import org.junit.Before
 import org.junit.Test
@@ -49,12 +46,25 @@ import java.io.IOException
  *
  * There is no JVM coverage of any of this: Build.VERSION.SDK_INT is 0 under
  * returnDefaultValues, so unit tests can only reach the routing function, never the writer.
+ *
+ * **Running this suite:** below Q the storage grant must exist before the app process
+ * forks (external storage is mounted at fork time), and Gradle's connectedAndroidTest
+ * installs and instruments with no window to grant in between - so these tests skip
+ * under it. Drive them manually on an API 26-28 device:
+ * ```
+ * ./gradlew :amethyst:assemblePlayDebug :amethyst:assemblePlayDebugAndroidTest
+ * adb install -r -g amethyst/build/outputs/apk/play/debug/amethyst-play-arm64-v8a-debug.apk
+ * adb install -r -g amethyst/build/outputs/apk/androidTest/play/debug/amethyst-play-debug-androidTest.apk
+ * adb shell am instrument -w -e class com.vitorpamplona.amethyst.ui.actions.MediaSaverToDiskLegacyStorageTest \
+ *     com.vitorpamplona.amethyst.debug.test/androidx.test.runner.AndroidJUnitRunner
+ * ```
  */
 @RunWith(AndroidJUnit4::class)
 class MediaSaverToDiskLegacyStorageTest {
     private val context get() = InstrumentationRegistry.getInstrumentation().targetContext
 
-    private val watchedDirs = listOf("Movies", "Pictures", "Download", "Music")
+    /** Every directory production can write to, straight from the routing table. */
+    private val watchedDirs = MediaSaverToDisk.MediaStoreTarget.entries.map { it.relativeDirectory }
     private val createdFiles = mutableListOf<File>()
 
     @Before
@@ -85,9 +95,8 @@ class MediaSaverToDiskLegacyStorageTest {
         // reaches it and every write fails with EACCES. Probe for real writability and
         // skip rather than report a routing failure that is really a harness problem.
         assumeTrue(
-            "External storage is not writable by this process. Below API 29 the grant must " +
-                "exist before the process starts - install with `adb install -r -g` and drive " +
-                "the run with `am instrument`; Gradle's connectedAndroidTest cannot grant in time.",
+            "External storage is not writable by this process; below API 29 the grant must " +
+                "exist at install time. See this class's KDoc for the exact run recipe.",
             canWriteToPublicStorage(),
         )
     }
@@ -96,7 +105,9 @@ class MediaSaverToDiskLegacyStorageTest {
         try {
             val dir = amethystDir("Movies").apply { if (!exists()) mkdirs() }
             val probe = File(dir, ".write-probe-${System.nanoTime()}")
-            probe.createNewFile().also { probe.delete() }
+            val writable = probe.createNewFile()
+            probe.delete()
+            writable
         } catch (e: IOException) {
             false
         }
@@ -128,26 +139,7 @@ class MediaSaverToDiskLegacyStorageTest {
     ) {
         val before = snapshot()
 
-        val localFile = File(context.cacheDir, "legacy-save-${System.nanoTime()}.bin")
-        localFile.writeBytes(ByteArray(2048) { it.toByte() })
-
-        var failure: Throwable? = null
-        var succeeded = false
-
-        runBlocking {
-            MediaSaverToDisk.save(
-                localFile = localFile,
-                mimeType = mimeType,
-                context = context,
-                onSuccess = { succeeded = true },
-                onError = { failure = it },
-            )
-        }
-
-        localFile.delete()
-
-        assertNull("save() reported an error: ${failure?.message}", failure)
-        assertTrue("save() never reported success", succeeded)
+        MediaSaverTestSupport.saveAndAssertSuccess(context, mimeType)
 
         val added = snapshot().mapValues { (dir, names) -> names - before.getValue(dir) }
         added.forEach { (dir, names) -> names.forEach { createdFiles.add(File(amethystDir(dir), it)) } }

amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/actions/MediaSaverToDiskMediaStoreTest.kt

@@ -21,41 +21,33 @@
 package com.vitorpamplona.amethyst.ui.actions
 
 import android.content.ContentResolver
+import android.content.ContentUris
 import android.net.Uri
 import android.os.Build
 import android.provider.MediaStore
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.platform.app.InstrumentationRegistry
-import kotlinx.coroutines.runBlocking
 import org.junit.After
 import org.junit.Assert.assertEquals
-import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Assume.assumeTrue
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
-import java.io.File
 
 /**
- * End-to-end regression test for issue #4009.
- *
- * MediaProvider validates RELATIVE_PATH's primary directory against the collection
- * being written to. Filing a video under "Pictures" threw
- * `IllegalArgumentException: Primary directory Pictures not allowed for
- * content://media/external/video/media; allowed directories are [DCIM, Movies]`
- * on Android 10; later releases accept the mismatch and silently misfile the video.
- *
- * This drives the real ContentResolver, so it catches both symptoms: the insert has
- * to succeed AND the row has to land in the directory the collection accepts.
+ * End-to-end regression test for issue #4009: drives the real ContentResolver, so it
+ * catches both symptoms of a collection/directory mismatch - Android 10 rejects the
+ * insert outright (the quoted rejection lives in [MediaSaverToDisk.MediaStoreTarget]'s
+ * KDoc), and later releases accept it and silently misfile the video.
  */
 @RunWith(AndroidJUnit4::class)
 class MediaSaverToDiskMediaStoreTest {
     private val context get() = InstrumentationRegistry.getInstrumentation().targetContext
     private val resolver: ContentResolver get() = context.contentResolver
 
-    /** Only rows this test inserted, identified by id in the collection they went into. */
-    private val created = mutableListOf<Pair<Uri, Long>>()
+    /** Only rows this test inserted, as item Uris in the collection they went into. */
+    private val created = mutableListOf<Uri>()
 
     @Before
     fun requiresScopedStorage() {
@@ -64,9 +56,7 @@ class MediaSaverToDiskMediaStoreTest {
 
     @After
     fun cleanUp() {
-        created.forEach { (collection, id) ->
-            resolver.delete(collection, "${MediaStore.MediaColumns._ID} = ?", arrayOf(id.toString()))
-        }
+        created.forEach { resolver.delete(it, null, null) }
     }
 
     @Test
@@ -91,27 +81,7 @@ class MediaSaverToDiskMediaStoreTest {
         // this suite is meant to be runnable on a real device holding real media.
         val highWaterMark = maxIdIn(collection)
 
-        val localFile = File(context.cacheDir, "media-saver-${System.nanoTime()}.bin")
-        localFile.writeBytes(ByteArray(2048) { it.toByte() })
-
-        var failure: Throwable? = null
-        var succeeded = false
-
-        runBlocking {
-            MediaSaverToDisk.save(
-                localFile = localFile,
-                mimeType = mimeType,
-                context = context,
-                onSuccess = { succeeded = true },
-                onError = { failure = it },
-            )
-        }
-
-        localFile.delete()
-
-        // Surfaces the #4009 IllegalArgumentException as the test failure message.
-        assertNull("save() reported an error: ${failure?.message}", failure)
-        assertTrue("save() never reported success", succeeded)
+        MediaSaverTestSupport.saveAndAssertSuccess(context, mimeType)
 
         return rowInsertedAfter(collection, highWaterMark)
     }
@@ -139,7 +109,7 @@ class MediaSaverToDiskMediaStoreTest {
                 "${MediaStore.MediaColumns._ID} ASC",
             )?.use { cursor ->
                 assertTrue("save() reported success but inserted no row into $collection", cursor.moveToFirst())
-                created.add(collection to cursor.getLong(0))
+                created.add(ContentUris.withAppendedId(collection, cursor.getLong(0)))
                 return cursor.getString(1)
             }
         return null

amethyst/src/main/java/com/vitorpamplona/amethyst/ui/actions/MediaSaverToDisk.kt

@@ -58,10 +58,11 @@ object MediaSaverToDisk {
         resolveBlossom: suspend (String) -> String? = { null },
         onSuccess: () -> Any?,
         onError: (Throwable) -> Any?,
-    ) = withContext(Dispatchers.IO) {
+    ) {
+        // No dispatch here: save() and downloadAndSave() both move themselves to IO.
         when {
             videoUri.isNullOrBlank() -> {
-                return@withContext
+                return
             }
 
             videoUri.startsWith("file") -> {
@@ -191,9 +192,9 @@ object MediaSaverToDisk {
 
     /**
      * Copies a local file into the gallery. Suspending and dispatched to IO like
-     * [downloadAndSave]: callers reach this from a click handler, and one of them
-     * (the storage-permission callback in FullScreenViewerChrome) launches on the
-     * main dispatcher, where copying a whole video would block the UI thread.
+     * [downloadAndSave]: callers reach this from click handlers, and a
+     * storage-permission callback among them launches on the main dispatcher,
+     * where copying a whole video would block the UI thread.
      */
     @OptIn(ExperimentalUuidApi::class)
     suspend fun save(
@@ -205,9 +206,6 @@ object MediaSaverToDisk {
     ) {
         withContext(Dispatchers.IO) {
             try {
-                val extension =
-                    mimeType?.let { MimeTypeMap.getSingleton().getExtensionFromMimeType(it) } ?: ""
-
                 // use{}: readAll leaves its source open, so without this the file
                 // descriptor stays open until the finalizer runs.
                 localFile.inputStream().source().buffer().use { buffer ->
@@ -219,6 +217,8 @@ object MediaSaverToDisk {
                             contentResolver = context.contentResolver,
                         )
                     } else {
+                        val extension =
+                            mimeType?.let { MimeTypeMap.getSingleton().getExtensionFromMimeType(it) } ?: ""
                         saveContentDefault(
                             fileName = "${Uuid.random()}.$extension",
                             contentType = mimeType ?: "",