feat: choose the default collection path at startup

Breaking change, this was previously done on first collection access.

Now this is set in AnkiDroidApp, and reading throws
StorageNotConfiguredException beforehand.

This contains a workaround for the OS-level bug: `SystemStorageException`
if the exception is caught, it is explicitly rethrown.

This is preparation for a full 'storage decision' screen during onboarding

* Part of 19552
* Part of 13574
* Part of 20737

Assisted-by: Claude Fable 5

Author: david-allison

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

@@ -57,6 +57,7 @@ import com.ichi2.anki.services.AlarmManagerService
 import com.ichi2.anki.services.NotificationService
 import com.ichi2.anki.settings.Prefs
 import com.ichi2.anki.settings.PrefsRepository
+import com.ichi2.anki.startup.ensureCollectionPathSet
 import com.ichi2.anki.startup.getDefaultAnkiDroidDirectory
 import com.ichi2.anki.ui.dialogs.ActivityAgnosticDialogs
 import com.ichi2.utils.ExceptionUtil
@@ -258,6 +259,7 @@ open class AnkiDroidApp :
             //  is not considered to be a fatal error, unless the directory itself is not writable.
             val ankiDroidDir =
                 try {
+                    ensureCollectionPathSet(this)
                     CollectionHelper.getCurrentAnkiDroidDirectory(this)
                 } catch (e: SystemStorageException) {
                     fatalInitializationError = FatalInitializationError.StorageError(e)

AnkiDroid/src/main/java/com/ichi2/anki/CollectionHelper.kt

@@ -11,12 +11,11 @@ import com.ichi2.anki.CollectionHelper.getCurrentAnkiDroidDirectory
 import com.ichi2.anki.common.preferences.sharedPrefs
 import com.ichi2.anki.common.utils.android.isInstrumentationTest
 import com.ichi2.anki.exception.StorageAccessException
+import com.ichi2.anki.exception.StorageNotConfiguredException
 import com.ichi2.anki.exception.SystemStorageException
 import com.ichi2.anki.libanki.Collection
 import com.ichi2.anki.libanki.CollectionFiles
-import com.ichi2.anki.startup.getDefaultAnkiDroidDirectory
 import com.ichi2.anki.storage.StorageDecision
-import com.ichi2.preferences.getOrSetString
 import timber.log.Timber
 import java.io.File
 import java.io.IOException
@@ -81,7 +80,8 @@ object CollectionHelper {
 
     /**
      * Try to access the current AnkiDroid directory
-     * @return whether or not dir is accessible
+     * @return whether or not dir is accessible: `false` if inaccessible, not yet configured,
+     * or the system could not provide a storage location
      * @param context to get directory with
      */
     fun isCurrentAnkiDroidDirAccessible(context: Context): Boolean =
@@ -91,6 +91,12 @@ object CollectionHelper {
         } catch (e: StorageAccessException) {
             Timber.w(e)
             false
+        } catch (e: StorageNotConfiguredException) {
+            Timber.w(e)
+            false
+        } catch (e: SystemStorageException) {
+            Timber.w(e)
+            false
         }
 
     /**
@@ -121,6 +127,17 @@ object CollectionHelper {
     /** A temporary override for [getCurrentAnkiDroidDirectory] */
     var ankiDroidDirectoryOverride: File? = null
 
+    /**
+     * Set when startup failed to choose a default collection path because Android could not
+     * provide a storage location ([SystemStorageException]: OS bug/SD card issue).
+     *
+     * Reads of an unset collection path rethrow this instead of [StorageNotConfiguredException],
+     * so the storage failure is not mistaken for the expected 'no collection path set' state.
+     */
+    // TODO: #19552 - consolidate with AnkiDroidApp.fatalError and StorageDecision (a dedicated
+    //  'storage unavailable' state) once the storage setup flow exists
+    var systemStorageFailure: SystemStorageException? = null
+
     /**
      * Whether the user has chosen where the collection is stored.
      *
@@ -133,49 +150,46 @@ object CollectionHelper {
     /**
      * @return the absolute path to the AnkiDroid directory.
      *
-     * @throws SystemStorageException if `getExternalFilesDir` returns null
+     * @throws StorageNotConfiguredException if no collection path has been set
+     * ([PREF_COLLECTION_PATH] is unset): a default is chosen during startup by
+     * [ensureCollectionPathSet][com.ichi2.anki.startup.ensureCollectionPathSet]
+     * @throws SystemStorageException if startup failed to choose a default collection path
+     * ([systemStorageFailure])
      */
-    fun getCurrentAnkiDroidDirectory(context: Context): File =
-        getCurrentAnkiDroidDirectoryOptionalContext(context.sharedPrefs()) { context }
+    fun getCurrentAnkiDroidDirectory(context: Context): File = getCurrentAnkiDroidDirectory(context.sharedPrefs())
 
     fun getCollectionPaths(context: Context): CollectionFiles = CollectionFiles.FolderBasedCollection(getCurrentAnkiDroidDirectory(context))
 
     // TODO: Duplicates collection.mediaFolder
     fun getMediaDirectory(context: Context) = getCollectionPaths(context).requireMediaFolder()
 
     /**
-     * An accessor which makes [Context] optional in the case that [PREF_COLLECTION_PATH] is set
-     *
      * @return the absolute path to the AnkiDroid directory.
+     *
+     * @throws StorageNotConfiguredException if no collection path has been set
+     * ([PREF_COLLECTION_PATH] is unset): a default is chosen during startup by
+     * [ensureCollectionPathSet][com.ichi2.anki.startup.ensureCollectionPathSet]
+     * @throws SystemStorageException if startup failed to choose a default collection path
+     * ([systemStorageFailure])
      */
-    // This uses a lambda as we typically depends on the `lateinit` appContext
-    // If we remove all Android references, we get a significant unit test speedup
-    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
-    internal fun getCurrentAnkiDroidDirectoryOptionalContext(
-        preferences: SharedPreferences,
-        context: () -> Context,
-    ): File =
-        if (isInstrumentationTest) {
+    fun getCurrentAnkiDroidDirectory(preferences: SharedPreferences): File {
+        val collectionDirectory = preferences.getString(PREF_COLLECTION_PATH, null)
+        return if (isInstrumentationTest) {
             // create an "androidTest" directory inside the current collection directory which contains the test data
             // "/AnkiDroid/androidTest" would be a new collection path
-            val currentCollectionDirectory =
-                preferences.getOrSetString(PREF_COLLECTION_PATH) {
-                    getDefaultAnkiDroidDirectory(context()).absolutePath
-                }
-            File(
-                currentCollectionDirectory,
-                "androidTest",
-            )
+            File(collectionDirectory ?: throw collectionPathUnset(), "androidTest")
         } else {
             ankiDroidDirectoryOverride
-                ?: File(
-                    preferences.getOrSetString(PREF_COLLECTION_PATH) {
-                        getDefaultAnkiDroidDirectory(
-                            context(),
-                        ).absolutePath
-                    },
-                )
+                ?: File(collectionDirectory ?: throw collectionPathUnset())
         }
+    }
+
+    /**
+     * The collection path is unset: either the expected pre-setup state
+     * ([StorageNotConfiguredException]) or startup failed to choose a default and the recorded
+     * [SystemStorageException] is rethrown.
+     */
+    private fun collectionPathUnset(): Exception = systemStorageFailure ?: StorageNotConfiguredException()
 
     /** Test-only override for [storageDecision]. @see ankiDroidDirectoryOverride */
     @VisibleForTesting

AnkiDroid/src/main/java/com/ichi2/anki/CollectionManager.kt

@@ -18,10 +18,10 @@ import com.ichi2.anki.CollectionManager.withCol
 import com.ichi2.anki.CollectionManager.withOpenColOrNull
 import com.ichi2.anki.CollectionManager.withQueue
 import com.ichi2.anki.backend.createDatabaseUsingRustBackend
-import com.ichi2.anki.common.android.appContext
 import com.ichi2.anki.common.utils.android.Threads
 import com.ichi2.anki.common.utils.android.isRobolectric
 import com.ichi2.anki.exception.StorageNotConfiguredException
+import com.ichi2.anki.exception.SystemStorageException
 import com.ichi2.anki.libanki.Collection
 import com.ichi2.anki.libanki.CollectionFiles
 import com.ichi2.anki.libanki.LibAnki
@@ -130,6 +130,8 @@ object CollectionManager {
      *
      * @throws StorageNotConfiguredException If [CollectionHelper.storageDecision] is undecided
      * (user has not selected a storage location).
+     * @throws SystemStorageException if startup failed to choose a default collection path
+     * ([CollectionHelper.systemStorageFailure])
      */
     suspend fun <T> withCol(
         @WorkerThread block: Collection.() -> T,
@@ -254,6 +256,8 @@ object CollectionManager {
      *
      * @throws StorageNotConfiguredException If [CollectionHelper.storageDecision] is undecided
      * (user has not selected a storage location).
+     * @throws SystemStorageException if startup failed to choose a default collection path
+     * ([CollectionHelper.systemStorageFailure])
      */
     suspend fun ensureOpen() {
         withQueue {
@@ -266,9 +270,15 @@ object CollectionManager {
      *
      * @throws StorageNotConfiguredException If [CollectionHelper.storageDecision] is not
      * [StorageDecision.Decided] (user has not selected a storage location).
+     * @throws SystemStorageException if startup failed to choose a default collection path
+     * ([CollectionHelper.systemStorageFailure])
      */
     private fun ensureOpenInner() {
-        if (CollectionHelper.storageDecision() != StorageDecision.Decided) throw StorageNotConfiguredException()
+        if (CollectionHelper.storageDecision() != StorageDecision.Decided) {
+            // rethrow a recorded startup storage failure: an OS bug/SD card issue must not be
+            // reported as the expected 'storage not configured' state
+            throw CollectionHelper.systemStorageFailure ?: StorageNotConfiguredException()
+        }
         ensureBackendInner()
         emulatedOpenFailure?.triggerFailure()
         if (collection == null || collection!!.dbClosed) {
@@ -290,8 +300,8 @@ object CollectionManager {
     }
 
     fun getCollectionDirectory() =
-        // Allow execution if appContext is not initialized
-        CollectionHelper.getCurrentAnkiDroidDirectoryOptionalContext(AnkiDroidApp.sharedPrefs()) { appContext }
+        // does not require appContext to be initialized
+        CollectionHelper.getCurrentAnkiDroidDirectory(AnkiDroidApp.sharedPrefs())
 
     /** Ensures the AnkiDroid directory is created, then returns the path to the
      * folder and the name of the collection file inside it. */
@@ -330,6 +340,8 @@ object CollectionManager {
      *
      * @throws StorageNotConfiguredException If [CollectionHelper.storageDecision] is undecided
      * (user has not selected a storage location).
+     * @throws SystemStorageException if startup failed to choose a default collection path
+     * ([CollectionHelper.systemStorageFailure])
      */
     fun getColUnsafe(): Collection =
         logUIHangs {

AnkiDroid/src/main/java/com/ichi2/anki/startup/SetupStorage.kt

@@ -8,12 +8,39 @@ import androidx.annotation.CheckResult
 import androidx.core.content.edit
 import com.ichi2.anki.CollectionHelper
 import com.ichi2.anki.common.preferences.sharedPrefs
+import com.ichi2.anki.exception.StorageNotConfiguredException
 import com.ichi2.anki.exception.SystemStorageException
 import com.ichi2.anki.selectAnkiDroidFolder
 import com.ichi2.anki.storage.AnkiDroidFolder
 import timber.log.Timber
 import java.io.File
 
+/**
+ * Ensures [CollectionHelper.PREF_COLLECTION_PATH] is set, choosing and persisting
+ * [getDefaultAnkiDroidDirectory] if it is unset.
+ *
+ * This decides the storage location on the user's behalf: until a storage setup flow exists
+ * (#19552), the user is not asked.
+ *
+ * @throws SystemStorageException if `getExternalFilesDir` returns null. The failure is recorded
+ * in [CollectionHelper.systemStorageFailure] so reads report it rather than
+ * [StorageNotConfiguredException]
+ */
+fun ensureCollectionPathSet(context: Context) {
+    val preferences = context.sharedPrefs()
+    if (preferences.contains(CollectionHelper.PREF_COLLECTION_PATH)) return
+    val defaultPath =
+        try {
+            getDefaultAnkiDroidDirectory(context).absolutePath
+        } catch (e: SystemStorageException) {
+            CollectionHelper.systemStorageFailure = e
+            throw e
+        }
+    Timber.i("collection path set to default")
+    Timber.d("default collection path: %s", defaultPath)
+    preferences.edit { putString(CollectionHelper.PREF_COLLECTION_PATH, defaultPath) }
+}
+
 /**
  * Get the absolute path to a directory that is suitable to be the default starting location
  * for the AnkiDroid directory.

AnkiDroid/src/test/java/com/ichi2/anki/StorageDecisionGateTest.kt

@@ -4,11 +4,13 @@ package com.ichi2.anki
 
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.ichi2.anki.exception.StorageNotConfiguredException
+import com.ichi2.anki.exception.SystemStorageException
 import com.ichi2.anki.storage.StorageDecision
 import org.junit.Test
 import org.junit.runner.RunWith
 import kotlin.test.assertEquals
 import kotlin.test.assertFailsWith
+import kotlin.test.assertSame
 
 /**
  * Proves the storage-decision gate in [CollectionManager.ensureOpenInner] is wired up. In production
@@ -38,4 +40,22 @@ class StorageDecisionGateTest : RobolectricTest() {
             CollectionHelper.storageDecisionTestOverride = null
         }
     }
+
+    /**
+     * A storage failure at startup ([SystemStorageException]: OS bug/SD card issue) must not
+     * masquerade as the expected 'storage not configured' state when opening the collection.
+     */
+    @Test
+    fun `opening the collection reports a recorded startup storage failure`() {
+        val failure = SystemStorageException.build("simulated getExternalFilesDir failure")
+        CollectionHelper.storageDecisionTestOverride = StorageDecision.Undecided
+        CollectionHelper.systemStorageFailure = failure
+        try {
+            val thrown = assertFailsWith<SystemStorageException> { CollectionManager.getColUnsafe() }
+            assertSame(failure, thrown)
+        } finally {
+            CollectionHelper.storageDecisionTestOverride = null
+            CollectionHelper.systemStorageFailure = null
+        }
+    }
 }

AnkiDroid/src/test/java/com/ichi2/anki/startup/SetupStorageTest.kt

@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package com.ichi2.anki.startup
+
+import androidx.core.content.edit
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.ichi2.anki.CollectionHelper
+import com.ichi2.anki.RobolectricTest
+import com.ichi2.anki.common.preferences.sharedPrefs
+import com.ichi2.anki.exception.StorageNotConfiguredException
+import com.ichi2.anki.exception.SystemStorageException
+import org.junit.After
+import org.junit.Test
+import org.junit.runner.RunWith
+import java.io.File
+import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
+import kotlin.test.assertSame
+import kotlin.test.assertTrue
+
+@RunWith(AndroidJUnit4::class)
+class SetupStorageTest : RobolectricTest() {
+    private val prefs
+        get() = targetContext.sharedPrefs()
+
+    private val collectionPath: String?
+        get() = prefs.getString(CollectionHelper.PREF_COLLECTION_PATH, null)
+
+    @After
+    fun resetCollectionHelperState() {
+        CollectionHelper.ankiDroidDirectoryOverride = null
+        CollectionHelper.systemStorageFailure = null
+    }
+
+    /**
+     * Reading the collection path must not silently choose and persist a default:
+     * that decision belongs to startup ([ensureCollectionPathSet]).
+     */
+    @Test
+    fun `reading the collection path throws when unset`() {
+        prefs.edit { remove(CollectionHelper.PREF_COLLECTION_PATH) }
+
+        assertFailsWith<StorageNotConfiguredException> {
+            CollectionHelper.getCurrentAnkiDroidDirectory(targetContext)
+        }
+    }
+
+    /**
+     * A storage failure at startup must not masquerade as the expected 'no collection path set'
+     * state: the [SystemStorageException] edge case (OS bug/SD card issue) is reported as itself.
+     */
+    @Test
+    fun `reading the collection path reports a recorded startup storage failure`() {
+        prefs.edit { remove(CollectionHelper.PREF_COLLECTION_PATH) }
+        val failure = SystemStorageException.build("simulated getExternalFilesDir failure")
+        CollectionHelper.systemStorageFailure = failure
+
+        val thrown =
+            assertFailsWith<SystemStorageException> {
+                CollectionHelper.getCurrentAnkiDroidDirectory(targetContext)
+            }
+        assertSame(failure, thrown)
+    }
+
+    @Test
+    fun `reading the collection path returns the stored value`() {
+        prefs.edit { putString(CollectionHelper.PREF_COLLECTION_PATH, "/a/collection/path") }
+
+        assertEquals(File("/a/collection/path"), CollectionHelper.getCurrentAnkiDroidDirectory(targetContext))
+    }
+
+    @Test
+    fun `the directory override takes precedence over the stored value`() {
+        prefs.edit { putString(CollectionHelper.PREF_COLLECTION_PATH, "/a/collection/path") }
+        CollectionHelper.ankiDroidDirectoryOverride = File("/an/override")
+
+        assertEquals(File("/an/override"), CollectionHelper.getCurrentAnkiDroidDirectory(targetContext))
+    }
+
+    @Test
+    fun `ensureCollectionPathSet persists a default when unset`() {
+        prefs.edit { remove(CollectionHelper.PREF_COLLECTION_PATH) }
+
+        ensureCollectionPathSet(targetContext)
+
+        assertTrue(!collectionPath.isNullOrEmpty(), "a default collection path should be set")
+    }
+
+    @Test
+    fun `ensureCollectionPathSet does not overwrite an existing value`() {
+        prefs.edit { putString(CollectionHelper.PREF_COLLECTION_PATH, "/a/custom/path") }
+
+        ensureCollectionPathSet(targetContext)
+
+        assertEquals("/a/custom/path", collectionPath)
+    }
+}