Skip to content

Commit 33b2d1a

Browse files
david-allisondavid-allison
committed
feat: storageDecision() is Undecided until a collection path is set
The decision now mirrors whether `getCurrentAnkiDroidDirectory` is usable or throws. The path is set during startup (`ensureCollectionPathSet`), so `Undecided` is only observable if startup storage initialization failed. This is now classified as an `InitializationError` and displayed in the app startup flow (with a TODO for 19552 to improve this). * Part of 19552 * Part of 13574 Assisted-by: Claude Fable 5
1 parent 080589e commit 33b2d1a

10 files changed

Lines changed: 124 additions & 36 deletions

File tree

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,13 @@ open class AnkiDroidApp :
533533
*/
534534
fun sharedPrefs() = sharedPreferencesTestingOverride ?: instance.sharedPrefs()
535535

536+
/**
537+
* [sharedPrefs], or `null` if unavailable: [instance] is not initialized when running
538+
* under a test-only [Application] (e.g. `EmptyApplication`) or in pure JVM tests
539+
*/
540+
fun sharedPrefsOrNull(): SharedPreferences? =
541+
sharedPreferencesTestingOverride ?: if (isInitialized) instance.sharedPrefs() else null
542+
536543
/** HACK: Whether an exception report has been thrown - TODO: Rewrite an ACRA Listener to do this */
537544
@VisibleForTesting
538545
var sentExceptionReportHack = false

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

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -139,13 +139,28 @@ object CollectionHelper {
139139
var systemStorageFailure: SystemStorageException? = null
140140

141141
/**
142-
* Whether the user has chosen where the collection is stored.
142+
* Whether the location of the collection has been decided: [StorageDecision.Decided] once
143+
* [PREF_COLLECTION_PATH] is set (or a [ankiDroidDirectoryOverride] is active), which mirrors
144+
* when [getCurrentAnkiDroidDirectory] can return a directory rather than throwing.
143145
*
144-
* TODO: real implementation based on whether [PREF_COLLECTION_PATH] is set.
145-
* TODO: What is a user revokes full storage?
146-
* This currently returns [StorageDecision.Decided], so callers that gate on it are no-ops.
146+
* The user is not asked yet: until the dedicated setup flow exists (#19552), the 'decision'
147+
* is made on their behalf during startup by
148+
* [ensureCollectionPathSet][com.ichi2.anki.startup.ensureCollectionPathSet], so this is
149+
* [StorageDecision.Decided] by the time the collection is opened.
150+
*
151+
* @param preferences the preferences the collection path will be read from: pass the same
152+
* (profile) context's preferences as the [getCurrentAnkiDroidDirectory] call being gated
153+
*
154+
* TODO: What if a user revokes full storage?
147155
*/
148-
fun storageDecision(): StorageDecision = storageDecisionTestOverride ?: StorageDecision.Decided
156+
fun storageDecision(preferences: SharedPreferences): StorageDecision {
157+
storageDecisionTestOverride?.let { return it }
158+
if (ankiDroidDirectoryOverride != null) return StorageDecision.Decided
159+
// a recorded systemStorageFailure also leaves the path unset: startup has already
160+
// surfaced it via AnkiDroidApp.fatalError, and reads rethrow it rather than
161+
// reporting 'not configured'
162+
return if (preferences.contains(PREF_COLLECTION_PATH)) StorageDecision.Decided else StorageDecision.Undecided
163+
}
149164

150165
/**
151166
* @return the absolute path to the AnkiDroid directory.

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import com.ichi2.anki.CollectionManager.withQueue
2020
import com.ichi2.anki.backend.createDatabaseUsingRustBackend
2121
import com.ichi2.anki.common.utils.android.Threads
2222
import com.ichi2.anki.common.utils.android.isRobolectric
23+
import com.ichi2.anki.common.utils.isRunningAsUnitTest
2324
import com.ichi2.anki.exception.StorageNotConfiguredException
2425
import com.ichi2.anki.exception.SystemStorageException
2526
import com.ichi2.anki.libanki.Collection
@@ -272,9 +273,21 @@ object CollectionManager {
272273
* [StorageDecision.Decided] (user has not selected a storage location).
273274
* @throws SystemStorageException if startup failed to choose a default collection path
274275
* ([CollectionHelper.systemStorageFailure])
276+
* @throws IllegalStateException if executed before `AnkiDroidApp.onCreate`
275277
*/
276278
private fun ensureOpenInner() {
277-
if (CollectionHelper.storageDecision() != StorageDecision.Decided) {
279+
// the decision is gated on the same preferences getCollectionDirectory() reads from
280+
val decision =
281+
CollectionHelper.storageDecisionTestOverride
282+
?: AnkiDroidApp.sharedPrefsOrNull()?.let { CollectionHelper.storageDecision(it) }
283+
// test-only: in-memory test collections have no preferences and read no path
284+
?: if (isRunningAsUnitTest) {
285+
StorageDecision.Decided
286+
} else {
287+
// the app instance is unset: treat this as a bug.
288+
throw IllegalStateException("Collection accessed before AnkiDroidApp was initialized")
289+
}
290+
if (decision != StorageDecision.Decided) {
278291
// rethrow a recorded startup storage failure: an OS bug/SD card issue must not be
279292
// reported as the expected 'storage not configured' state
280293
throw CollectionHelper.systemStorageFailure ?: StorageNotConfiguredException()

‎AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1002,6 +1002,9 @@ open class DeckPicker :
10021002
override val requiredPermissions: PermissionSet
10031003
get() = permissions
10041004

1005+
override val preferences: SharedPreferences
1006+
get() = context.sharedPrefs()
1007+
10051008
override fun initializeAnkiDroidFolder(): Boolean = CollectionHelper.isCurrentAnkiDroidDirAccessible(context)
10061009
}
10071010

@@ -1035,9 +1038,16 @@ open class DeckPicker :
10351038
is DiskFull -> displayNoStorageError()
10361039
is DBError -> displayDatabaseFailure(CustomExceptionData.fromException(failure.exception))
10371040
is StorageUndecided -> {
1038-
Timber.i("Displaying storage setup required")
1039-
// unreachable: storageDecision() cannot yet return Undecided outside tests
1040-
TODO("#19552 - replace with the storage setup flow.")
1041+
// unreachable: Undecided requires PREF_COLLECTION_PATH to be unset, which only
1042+
// happens if ensureCollectionPathSet failed at startup; getStartupFailureType
1043+
// then returns InitializationError (fatalError) before checking the decision
1044+
// TODO: #19552 - replace with the storage setup flow
1045+
Timber.w("storage setup flow (#19552) not implemented; showing load-failure options")
1046+
CrashReportService.sendExceptionReport(
1047+
IllegalStateException("StorageUndecided reached without a startup failure"),
1048+
"DeckPicker::handleStartupFailure",
1049+
)
1050+
showDatabaseErrorDialog(DatabaseErrorDialogType.DIALOG_LOAD_FAILED)
10411051
}
10421052
}
10431053
}

‎AnkiDroid/src/main/java/com/ichi2/anki/InitialActivity.kt‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import androidx.core.content.edit
3030
import com.ichi2.anki.backend.DatabaseCorruption
3131
import com.ichi2.anki.common.crashreporting.CrashReportService
3232
import com.ichi2.anki.common.permissions.hasAllPermissions
33+
import com.ichi2.anki.common.preferences.sharedPrefs
3334
import com.ichi2.anki.common.utils.android.SdCard
3435
import com.ichi2.anki.compat.CompatHelper.Companion.sdkVersion
3536
import com.ichi2.anki.exception.StorageAccessException
@@ -53,18 +54,21 @@ import timber.log.Timber
5354
object InitialActivity {
5455
@CheckResult
5556
fun getStartupFailureType(context: Context): StartupFailure? =
56-
getStartupFailureType { CollectionHelper.isCurrentAnkiDroidDirAccessible(context) }
57+
getStartupFailureType(context.sharedPrefs()) { CollectionHelper.isCurrentAnkiDroidDirAccessible(context) }
5758

5859
/** Returns null on success */
5960
@CheckResult
60-
fun getStartupFailureType(initializeAnkiDroidDirectory: () -> Boolean): StartupFailure? {
61+
fun getStartupFailureType(
62+
preferences: SharedPreferences,
63+
initializeAnkiDroidDirectory: () -> Boolean,
64+
): StartupFailure? {
6165
AnkiDroidApp.fatalError?.let {
6266
return StartupFailure.InitializationError(it)
6367
}
6468

6569
// Opening the collection would throw a StorageNotConfiguredException, which is not worth
6670
// a crash report
67-
if (CollectionHelper.storageDecision() != StorageDecision.Decided) {
71+
if (CollectionHelper.storageDecision(preferences) != StorageDecision.Decided) {
6872
Timber.i("storage location is undecided")
6973
return StartupFailure.StorageUndecided
7074
}

‎AnkiDroid/src/main/java/com/ichi2/anki/IntentHandler.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import com.ichi2.anki.common.annotations.NeedsTest
1818
import com.ichi2.anki.common.coroutines.applicationScope
1919
import com.ichi2.anki.common.permissions.hasLegacyStorageAccessPermission
2020
import com.ichi2.anki.common.permissions.isExternalStorageManagerCompat
21+
import com.ichi2.anki.common.preferences.sharedPrefs
2122
import com.ichi2.anki.common.utils.android.showThemedToast
2223
import com.ichi2.anki.common.utils.trimToLength
2324
import com.ichi2.anki.dialogs.DialogHandler.Companion.storeMessage
@@ -134,7 +135,7 @@ class IntentHandler : AbstractIntentHandler() {
134135
action: String?,
135136
block: () -> Unit,
136137
) {
137-
if (CollectionHelper.storageDecision() != StorageDecision.Decided) {
138+
if (CollectionHelper.storageDecision(sharedPrefs()) != StorageDecision.Decided) {
138139
// checked before permissions: the permissions required depend on the chosen folder
139140
Timber.i("Storage is not configured, cancelling intent '%s'", action)
140141
launchDeckPickerIfNoOtherTasks(reloadIntent)

‎AnkiDroid/src/main/java/com/ichi2/anki/deckpicker/DeckPickerViewModel.kt‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,7 @@ class DeckPickerViewModel :
522522
}
523523

524524
Timber.d("handleStartup: Continuing after permission granted")
525-
val failure = InitialActivity.getStartupFailureType(environment::initializeAnkiDroidFolder)
525+
val failure = InitialActivity.getStartupFailureType(environment.preferences, environment::initializeAnkiDroidFolder)
526526
if (failure != null) {
527527
flowOfStartupResponse.value = StartupResponse.FatalError(failure)
528528
return
@@ -540,6 +540,9 @@ class DeckPickerViewModel :
540540

541541
val requiredPermissions: PermissionSet
542542

543+
/** The preferences of the (profile) context the collection path is read from */
544+
val preferences: SharedPreferences
545+
543546
fun initializeAnkiDroidFolder(): Boolean
544547
}
545548

‎AnkiDroid/src/main/java/com/ichi2/anki/startup/StartupGuards.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import android.app.Activity
66
import android.content.Intent
77
import com.ichi2.anki.CollectionHelper
88
import com.ichi2.anki.IntentHandler
9+
import com.ichi2.anki.common.preferences.sharedPrefs
910
import com.ichi2.anki.exception.SystemStorageException
1011
import com.ichi2.anki.storage.StorageDecision
1112
import timber.log.Timber
@@ -45,7 +46,7 @@ fun Activity.ensureStorageIsReady(): Boolean {
4546
* @see redirectToMainEntryPoint
4647
*/
4748
private fun Activity.ensureStorageIsConfigured(): Boolean {
48-
if (CollectionHelper.storageDecision() == StorageDecision.Decided) {
49+
if (CollectionHelper.storageDecision(sharedPrefs()) == StorageDecision.Decided) {
4950
return true
5051
}
5152
Timber.w("finishing activity. Storage is not configured")

‎AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,18 @@ class DeckPickerTest : RobolectricTest() {
254254
)
255255
}
256256

257+
/** Until the storage setup flow exists (#19552), the user gets recovery options, not a crash */
258+
@Test
259+
fun `storage undecided shows load-failure options rather than crashing`() {
260+
// don't call .onCreate
261+
val deckPicker = Robolectric.buildActivity(DeckPickerEx::class.java, Intent()).get()
262+
deckPicker.handleStartupFailure(InitialActivity.StartupFailure.StorageUndecided)
263+
assertThat(
264+
deckPicker.databaseErrorDialog,
265+
equalTo(DatabaseErrorDialogType.DIALOG_LOAD_FAILED),
266+
)
267+
}
268+
257269
@Test
258270
fun databaseLockedWithPermissionIntegrationTest() {
259271
AnkiDroidApp.sentExceptionReportHack = false

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

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,43 +2,69 @@
22

33
package com.ichi2.anki
44

5+
import androidx.core.content.edit
56
import androidx.test.ext.junit.runners.AndroidJUnit4
7+
import com.ichi2.anki.common.preferences.sharedPrefs
68
import com.ichi2.anki.exception.StorageNotConfiguredException
79
import com.ichi2.anki.exception.SystemStorageException
810
import com.ichi2.anki.storage.StorageDecision
11+
import org.junit.After
912
import org.junit.Test
1013
import org.junit.runner.RunWith
14+
import java.io.File
1115
import kotlin.test.assertEquals
1216
import kotlin.test.assertFailsWith
1317
import kotlin.test.assertSame
1418

1519
/**
16-
* Proves the storage-decision gate in [CollectionManager.ensureOpenInner] is wired up. In production
17-
* [CollectionHelper.storageDecision] always returns [com.ichi2.anki.storage.StorageDecision.Decided], so this never fires;
18-
* here we force [com.ichi2.anki.storage.StorageDecision.Undecided] via the test override.
20+
* Proves the storage-decision gate in [CollectionManager.ensureOpenInner] is wired up.
21+
* [CollectionHelper.storageDecision] is [StorageDecision.Decided] once a collection path has
22+
* been set ([CollectionHelper.PREF_COLLECTION_PATH]); tests force
23+
* [StorageDecision.Undecided] via the test override.
1924
*/
2025
@RunWith(AndroidJUnit4::class)
2126
class StorageDecisionGateTest : RobolectricTest() {
27+
private val prefs
28+
get() = targetContext.sharedPrefs()
29+
30+
@After
31+
fun resetOverrides() {
32+
CollectionHelper.ankiDroidDirectoryOverride = null
33+
CollectionHelper.storageDecisionTestOverride = null
34+
CollectionHelper.systemStorageFailure = null
35+
}
36+
37+
@Test
38+
fun `storage decision is decided when the collection path is set`() {
39+
prefs.edit { putString(CollectionHelper.PREF_COLLECTION_PATH, "/a/collection/path") }
40+
assertEquals(StorageDecision.Decided, CollectionHelper.storageDecision(prefs))
41+
}
42+
43+
@Test
44+
fun `storage decision is undecided when the collection path is unset`() {
45+
prefs.edit { remove(CollectionHelper.PREF_COLLECTION_PATH) }
46+
assertEquals(StorageDecision.Undecided, CollectionHelper.storageDecision(prefs))
47+
}
48+
49+
@Test
50+
fun `storage decision is decided when a directory override is active`() {
51+
prefs.edit { remove(CollectionHelper.PREF_COLLECTION_PATH) }
52+
CollectionHelper.ankiDroidDirectoryOverride = File("/an/override")
53+
assertEquals(StorageDecision.Decided, CollectionHelper.storageDecision(prefs))
54+
}
55+
2256
@Test
2357
fun `opening the collection throws when storage is undecided`() {
2458
CollectionHelper.storageDecisionTestOverride = StorageDecision.Undecided
25-
try {
26-
assertFailsWith<StorageNotConfiguredException> { CollectionManager.getColUnsafe() }
27-
} finally {
28-
CollectionHelper.storageDecisionTestOverride = null
29-
}
59+
assertFailsWith<StorageNotConfiguredException> { CollectionManager.getColUnsafe() }
3060
}
3161

3262
/** No collection access should be attempted: a crash report would otherwise be generated */
3363
@Test
3464
fun `startup failure is StorageUndecided when storage is undecided`() {
3565
CollectionHelper.storageDecisionTestOverride = StorageDecision.Undecided
36-
try {
37-
val failure = InitialActivity.getStartupFailureType { true }
38-
assertEquals(InitialActivity.StartupFailure.StorageUndecided, failure)
39-
} finally {
40-
CollectionHelper.storageDecisionTestOverride = null
41-
}
66+
val failure = InitialActivity.getStartupFailureType(prefs) { true }
67+
assertEquals(InitialActivity.StartupFailure.StorageUndecided, failure)
4268
}
4369

4470
/**
@@ -50,12 +76,8 @@ class StorageDecisionGateTest : RobolectricTest() {
5076
val failure = SystemStorageException.build("simulated getExternalFilesDir failure")
5177
CollectionHelper.storageDecisionTestOverride = StorageDecision.Undecided
5278
CollectionHelper.systemStorageFailure = failure
53-
try {
54-
val thrown = assertFailsWith<SystemStorageException> { CollectionManager.getColUnsafe() }
55-
assertSame(failure, thrown)
56-
} finally {
57-
CollectionHelper.storageDecisionTestOverride = null
58-
CollectionHelper.systemStorageFailure = null
59-
}
79+
80+
val thrown = assertFailsWith<SystemStorageException> { CollectionManager.getColUnsafe() }
81+
assertSame(failure, thrown)
6082
}
6183
}

0 commit comments

Comments
 (0)