Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/anki/CardTemplateEditor.kt
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ import com.ichi2.anki.previewer.TemplatePreviewerFragment
import com.ichi2.anki.previewer.TemplatePreviewerPage
import com.ichi2.anki.settings.Prefs
import com.ichi2.anki.snackbar.showSnackbar
import com.ichi2.anki.startup.ensureStorageIsReady
import com.ichi2.anki.ui.ResizablePaneManager
import com.ichi2.anki.ui.internationalization.sentenceCase
import com.ichi2.anki.utils.ext.dismissAllDialogFragments
Expand Down Expand Up @@ -184,6 +185,9 @@ open class CardTemplateEditor : AnkiActivity(R.layout.activity_card_template_edi
return
}
super.onCreate(savedInstanceState)
if (!ensureStorageIsReady()) {
return
}
// Load the args either from the intent or savedInstanceState bundle
if (savedInstanceState == null) {
// get note type id
Expand Down
8 changes: 8 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@ import com.ichi2.anki.common.destinations.DeckOptionsDestination
import com.ichi2.anki.dialogs.DatabaseErrorDialog
import com.ichi2.anki.dialogs.DatabaseErrorDialog.DatabaseErrorDialogType
import com.ichi2.anki.exception.StorageAccessException
import com.ichi2.anki.exception.StorageNotConfiguredException
import com.ichi2.anki.libanki.exception.InvalidSearchException
import com.ichi2.anki.pages.fromCurrentDeck
import com.ichi2.anki.pages.toIntent
import com.ichi2.anki.snackbar.showSnackbar
import com.ichi2.anki.startup.redirectToMainEntryPoint
import com.ichi2.anki.ui.internationalization.sentenceCase
import com.ichi2.anki.utils.openUrl
import com.ichi2.utils.create
Expand Down Expand Up @@ -190,6 +192,12 @@ suspend fun <T> FragmentActivity.runCatching(
Timber.w(exc, errorMessage)
exc.localizedMessage?.let { showSnackbar(it) }
}
is StorageNotConfiguredException -> {
// expected before first-run setup completes: no crash report
// edge case when 'ensureStorageIsReady' was insufficient
Timber.w(exc, errorMessage)
if (!isFinishing) redirectToMainEntryPoint()
}
is BackendNetworkException, is BackendSyncException, is StorageAccessException, is BackendCardTypeException -> {
// these exceptions do not generate worthwhile crash reports
Timber.i("Showing error dialog but not sending a crash report.")
Expand Down
6 changes: 6 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ import com.ichi2.anki.InitialActivity.StartupFailure.DirectoryNotAccessible
import com.ichi2.anki.InitialActivity.StartupFailure.DiskFull
import com.ichi2.anki.InitialActivity.StartupFailure.FutureAnkidroidVersion
import com.ichi2.anki.InitialActivity.StartupFailure.SDCardNotMounted
import com.ichi2.anki.InitialActivity.StartupFailure.StorageUndecided
import com.ichi2.anki.IntentHandler.Companion.intentToReviewDeckFromShortcuts
import com.ichi2.anki.account.AccountActivity
import com.ichi2.anki.analytics.UsageAnalytics
Expand Down Expand Up @@ -1025,6 +1026,11 @@ open class DeckPicker :
is StartupFailure.InitializationError -> FatalErrorDialog.build(this, failure).show()
is DiskFull -> displayNoStorageError()
is DBError -> displayDatabaseFailure(CustomExceptionData.fromException(failure.exception))
is StorageUndecided -> {
Timber.i("Displaying storage setup required")
// unreachable: storageDecision() cannot yet return Undecided outside tests
TODO("#19552 - replace with the storage setup flow.")
}
}
}

Expand Down
15 changes: 15 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/anki/InitialActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import com.ichi2.anki.servicelayer.PreferenceUpgradeService
import com.ichi2.anki.servicelayer.PreferenceUpgradeService.setPreferencesUpToDate
import com.ichi2.anki.servicelayer.ScopedStorageService.isLegacyStorage
import com.ichi2.anki.storage.AnkiDroidFolder
import com.ichi2.anki.storage.StorageDecision
import com.ichi2.anki.ui.windows.permissions.InternetPermissionFragment
import com.ichi2.anki.ui.windows.permissions.NotificationsPermissionFragment
import com.ichi2.anki.ui.windows.permissions.PermissionsFragment
Expand All @@ -61,6 +62,13 @@ object InitialActivity {
return StartupFailure.InitializationError(it)
}

// Opening the collection would throw a StorageNotConfiguredException, which is not worth
// a crash report
if (CollectionHelper.storageDecision() != StorageDecision.Decided) {
Timber.i("storage location is undecided")
return StartupFailure.StorageUndecided
}

val failure =
try {
CollectionManager.getColUnsafe()
Expand Down Expand Up @@ -187,6 +195,13 @@ object InitialActivity {
}

data object DiskFull : StartupFailure()

/**
* The user has not yet decided where the collection is stored.
*
* @see CollectionHelper.storageDecision
*/
data object StorageUndecided : StartupFailure()
}
}

Expand Down
9 changes: 9 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/anki/IntentHandler.kt
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import com.ichi2.anki.libanki.DeckId
import com.ichi2.anki.noteeditor.NoteEditorLauncher
import com.ichi2.anki.servicelayer.ScopedStorageService
import com.ichi2.anki.settings.Prefs
import com.ichi2.anki.storage.StorageDecision
import com.ichi2.anki.ui.windows.reviewer.ReviewerFragment
import com.ichi2.anki.utils.MimeTypeUtils
import com.ichi2.anki.worker.SyncWorker
Expand Down Expand Up @@ -124,13 +125,21 @@ class IntentHandler : AbstractIntentHandler() {
* * AnkiDroid is using a legacy directory to store user data but has access to it since storage permission
* has been granted (as long as AnkiDroid targeted API < 30, requested legacy storage, and has not been uninstalled since)
*
* The user must also have [decided][CollectionHelper.storageDecision] where data is stored;
* otherwise they are routed to the [DeckPicker].
*/
@NeedsTest("clicking a file in 'Files' to import")
private fun performActionIfStorageAccessible(
reloadIntent: Intent,
action: String?,
block: () -> Unit,
) {
if (CollectionHelper.storageDecision() != StorageDecision.Decided) {
// checked before permissions: the permissions required depend on the chosen folder
Timber.i("Storage is not configured, cancelling intent '%s'", action)
launchDeckPickerIfNoOtherTasks(reloadIntent)
return
}
if (grantedStoragePermissions(this, showToast = true)) {
Timber.i("User has storage permissions. Running intent: %s", action)
block()
Expand Down
4 changes: 4 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/anki/NoteTypeFieldEditor.kt
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import com.ichi2.anki.libanki.NotetypeJson
import com.ichi2.anki.libanki.exception.ConfirmModSchemaException
import com.ichi2.anki.servicelayer.LanguageHintService.setLanguageHintForField
import com.ichi2.anki.snackbar.showSnackbar
import com.ichi2.anki.startup.ensureStorageIsReady
import com.ichi2.anki.ui.internationalization.sentenceCase
import com.ichi2.anki.utils.ext.dismissAllDialogFragments
import com.ichi2.anki.utils.ext.setCompoundDrawablesRelativeWithIntrinsicBoundsKt
Expand Down Expand Up @@ -97,6 +98,9 @@ class NoteTypeFieldEditor : AnkiActivity(R.layout.activity_note_type_field_edito
return
}
super.onCreate(savedInstanceState)
if (!ensureStorageIsReady()) {
return
}
setContentView(R.layout.activity_note_type_field_editor)
enableToolbar()
binding.notetypeName.text = intent.getStringExtra(EXTRA_NOTETYPE_NAME)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.CustomStudyAction.Co
import com.ichi2.anki.libanki.undoAvailable
import com.ichi2.anki.libanki.undoLabel
import com.ichi2.anki.observability.ChangeManager
import com.ichi2.anki.startup.ensureStorageIsReady
import com.ichi2.anki.utils.ext.setFragmentResultListener
import com.ichi2.ui.RtlCompliantActionProvider
import kotlinx.coroutines.launch
Expand All @@ -47,6 +48,9 @@ class StudyOptionsActivity :
return
}
super.onCreate(savedInstanceState)
if (!ensureStorageIsReady()) {
return
}
enableToolbar().apply { title = "" }
if (savedInstanceState == null) {
loadStudyOptionsFragment()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import com.ichi2.anki.multimediacard.IMultimediaEditableNote
import com.ichi2.anki.multimediacard.fields.IField
import com.ichi2.anki.snackbar.BaseSnackbarBuilderProvider
import com.ichi2.anki.snackbar.SnackbarBuilder
import com.ichi2.anki.startup.ensureStorageIsReady
import com.ichi2.themes.setTransparentStatusBar
import com.ichi2.utils.FragmentFactoryUtils
import dev.androidbroadcast.vbpd.viewBinding
Expand Down Expand Up @@ -75,6 +76,9 @@ class MultimediaActivity :
return
}
super.onCreate(savedInstanceState)
if (!ensureStorageIsReady()) {
return
}
setTransparentStatusBar()
setSupportActionBar(binding.toolbar)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import com.ichi2.anki.dialogs.showLoadingDialog
import com.ichi2.anki.launchCatchingTask
import com.ichi2.anki.notetype.ManageNoteTypesState.UserMessage
import com.ichi2.anki.snackbar.showSnackbar
import com.ichi2.anki.startup.ensureStorageIsReady
import com.ichi2.anki.sync.userAcceptsSchemaChange
import com.ichi2.anki.utils.Destination
import com.ichi2.themes.setTransparentStatusBar
Expand Down Expand Up @@ -97,6 +98,9 @@ class ManageNotetypes : AnkiActivity(R.layout.activity_manage_note_types) {
return
}
super.onCreate(savedInstanceState)
if (!ensureStorageIsReady()) {
return
}

setTransparentStatusBar()
enableToolbar()
Expand Down
11 changes: 7 additions & 4 deletions AnkiDroid/src/main/java/com/ichi2/anki/startup/StartupGuards.kt
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,20 @@ import com.ichi2.anki.storage.StorageDecision
import timber.log.Timber

/**
* Ensures storage is ready for use, finishes the activity is otherwise.
* Ensures storage is ready for use, finishes the activity and calls [redirectToMainEntryPoint]
* otherwise.
*
* - the user has decided where the collection is stored
* - the app has permission to access
* - the user has decided where the collection is stored. ([ensureStorageIsConfigured])
* - the app has permission to access the collection. ([ensureStoragePermissions])
*
* This should be called AFTER a call to `super.`[onCreate][Activity.onCreate]
*
* @return `true`: activity may continue to start, `false`: [onCreate][Activity.onCreate]
* should stop executing
*
* @throws SystemStorageException if `getExternalFilesDir` returns null
* @throws SystemStorageException if `getExternalFilesDir` returns `null`
*
* @see redirectToMainEntryPoint
*/
fun Activity.ensureStorageIsReady(): Boolean {
// The storage decision is checked first
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// SPDX-License-Identifier: GPL-3.0-or-later

package com.ichi2.anki

import com.ichi2.anki.storage.StorageDecision
import com.ichi2.testutils.ActivityList
import com.ichi2.testutils.ActivityList.ActivityLaunchParam
import org.junit.After
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.ParameterizedRobolectricTestRunner

/**
* All activities start crash-free when [CollectionHelper.storageDecision] returns
* [StorageDecision.Undecided].
*
* Unlike [ExternalEntryPointsUndecidedStorageTest], this covers in-app navigation and pinned
* shortcuts.
*
* Collection-requiring activities are expected to finish via [ensureStorageIsReady][com.ichi2.anki.startup.ensureStorageIsReady].
*
* If this fails for a new activity, add the following after `super.onCreate`:
* ```
* if (!ensureStorageIsReady()) {
* return
* }
* ```
*/
@RunWith(ParameterizedRobolectricTestRunner::class)
class ActivityStartupUndecidedStorageTest : RobolectricTest() {
@ParameterizedRobolectricTestRunner.Parameter
@JvmField // required for Parameter
var launcher: ActivityLaunchParam? = null

// Only used for display, but needs to be defined
@ParameterizedRobolectricTestRunner.Parameter(1)
@JvmField // required for Parameter
@Suppress("unused")
var activityName: String? = null

@Before
fun setStorageUndecided() {
CollectionHelper.storageDecisionTestOverride = StorageDecision.Undecided
}

@After
fun resetStorageDecision() {
CollectionHelper.storageDecisionTestOverride = null
}

@Test
fun `startup is crash-free when storage is undecided`() {
val controller = launcher!!.build(targetContext)
// mirrors Android: an activity which finishes during onCreate (e.g. redirectToMainEntryPoint)
// does not receive the remaining lifecycle callbacks; controller.setup() would force them
controller.create()
if (!controller.get().isFinishing) {
controller
.start()
.postCreate(null)
.resume()
.visible()
}
advanceRobolectricLooper()
}

companion object {
@Suppress("unused")
@ParameterizedRobolectricTestRunner.Parameters(name = "{1}")
@JvmStatic // required for initParameters
fun initParameters(): Collection<Array<Any>> = ActivityList.allActivitiesAndIntents().map { arrayOf(it, it.simpleName) }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// SPDX-License-Identifier: GPL-3.0-or-later

package com.ichi2.anki

import androidx.fragment.app.FragmentActivity
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.ichi2.anki.exception.StorageNotConfiguredException
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.Robolectric
import org.robolectric.Shadows.shadowOf
import kotlin.test.assertEquals
import kotlin.test.assertNotNull
import kotlin.test.assertTrue

@RunWith(AndroidJUnit4::class)
class CoroutineHelpersRobolectricTest : RobolectricTest() {
/**
* A [StorageNotConfiguredException] escaping a coroutine means an activity raced or
* outlived its [ensureStorageIsReady][com.ichi2.anki.startup.ensureStorageIsReady] check:
* the user should be sent to the main entry point, which handles storage setup.
Comment on lines +19 to +21

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function is ensureStorageIsReady

*/
@Test
fun `launchCatchingTask redirects to main entry point when storage is not configured`() {
val activity = Robolectric.buildActivity(FragmentActivity::class.java).create().get()

activity.launchCatchingTask { throw StorageNotConfiguredException() }
advanceRobolectricLooper()

assertTrue(activity.isFinishing, "activity should finish")
val redirect = shadowOf(activity).nextStartedActivity
assertNotNull(redirect, "the main entry point should be opened")
assertEquals(IntentHandler::class.qualifiedName, redirect.component?.className)
}
}
13 changes: 13 additions & 0 deletions AnkiDroid/src/test/java/com/ichi2/anki/StorageDecisionGateTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import com.ichi2.anki.exception.StorageNotConfiguredException
import com.ichi2.anki.storage.StorageDecision
import org.junit.Test
import org.junit.runner.RunWith
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith

/**
Expand All @@ -25,4 +26,16 @@ class StorageDecisionGateTest : RobolectricTest() {
CollectionHelper.storageDecisionTestOverride = null
}
}

/** No collection access should be attempted: a crash report would otherwise be generated */
@Test
fun `startup failure is StorageUndecided when storage is undecided`() {
CollectionHelper.storageDecisionTestOverride = StorageDecision.Undecided
try {
val failure = InitialActivity.getStartupFailureType { true }
assertEquals(InitialActivity.StartupFailure.StorageUndecided, failure)
} finally {
CollectionHelper.storageDecisionTestOverride = null
}
}
}
Loading