From bdf3af8e76033f31209405b85038e9354d80fe36 Mon Sep 17 00:00:00 2001 From: David Allison <62114487+david-allison@users.noreply.github.com> Date: Sun, 16 Aug 2026 16:18:05 +0100 Subject: [PATCH 1/2] refactor(permissions): derive APP_PRIVATE from the collection path A `full` build using app-private storage will no longer require `MANAGE_EXTERNAL_STORAGE` permissions (skipping a check checking device capabilities. No difference on a `play` variant. Prep for 13574 Part of 21046: 'Skip' will select app-private storage, and this check is what keeps the permission screen closed afterwards. Co-authored-by: Fandroid745 Assisted-by: Claude Fable 5 --- .../java/com/ichi2/anki/InitialActivity.kt | 9 ++- .../anki/SelectStoragePermissionsTest.kt | 57 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/InitialActivity.kt b/AnkiDroid/src/main/java/com/ichi2/anki/InitialActivity.kt index e7cd1545f14a..a7388cc2dd3e 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/InitialActivity.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/InitialActivity.kt @@ -243,8 +243,15 @@ internal fun selectStoragePermissions( } fun selectStoragePermissions(context: Context): PermissionSet { + // `false`: the collection is app-private, so it can be accessed without storage permissions + // `null`: no collection path is set + val currentFolderIsLegacy = isLegacyStorage(context, setCollectionPath = false) + if (currentFolderIsLegacy == false) { + return PermissionSet.APP_PRIVATE + } + val canAccessLegacyStorage = Build.VERSION.SDK_INT < Build.VERSION_CODES.Q || Environment.isExternalStorageLegacy() - val currentFolderIsAccessibleAndLegacy = canAccessLegacyStorage && isLegacyStorage(context, setCollectionPath = false) == true + val currentFolderIsAccessibleAndLegacy = canAccessLegacyStorage && currentFolderIsLegacy == true return selectStoragePermissions( canManageExternalStorage = Permissions.canManageExternalStorage(context), diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/SelectStoragePermissionsTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/SelectStoragePermissionsTest.kt index 52b5f3a6b38b..49b8dbef8fde 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/SelectStoragePermissionsTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/SelectStoragePermissionsTest.kt @@ -3,9 +3,18 @@ package com.ichi2.anki import android.annotation.SuppressLint +import android.content.Context import android.os.Build +import androidx.core.content.edit +import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.ichi2.anki.common.preferences.sharedPrefs +import com.ichi2.anki.common.storage.CollectionHelper import com.ichi2.testutils.EmptyApplication +import com.ichi2.utils.Permissions +import io.mockk.every +import io.mockk.mockkObject +import io.mockk.unmockkObject import org.hamcrest.CoreMatchers.equalTo import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.contains @@ -13,6 +22,7 @@ import org.junit.Test import org.junit.experimental.categories.Category import org.junit.runner.RunWith import org.robolectric.annotation.Config +import java.io.File import kotlin.test.assertTrue /** @@ -95,6 +105,53 @@ class SelectStoragePermissionsTest { ) } + @SuppressLint("NewApi") // EXTERNAL_MANAGER requires R, guaranteed by @Config + @Config(sdk = [R_OR_AFTER]) + @Test // #13574: no collection path is set: permissions are based on device capabilities + fun `full build - screen is required while no collection path is set`() { + context.sharedPrefs().edit { remove(CollectionHelper.PREF_COLLECTION_PATH) } + withManageExternalStorageInManifest { + assertThat(selectStoragePermissions(context), equalTo(PermissionSet.EXTERNAL_MANAGER)) + } + } + + @SuppressLint("NewApi") // EXTERNAL_MANAGER requires R, guaranteed by @Config + @Config(sdk = [R_OR_AFTER]) + @Test // #13574: public storage which the app cannot access: the screen is required + fun `full build - screen is required when access to public storage was revoked`() { + context.sharedPrefs().edit { + putString(CollectionHelper.PREF_COLLECTION_PATH, "/storage/emulated/0/AnkiDroid") + } + withManageExternalStorageInManifest { + assertThat(selectStoragePermissions(context), equalTo(PermissionSet.EXTERNAL_MANAGER)) + } + } + + @Config(sdk = [R_OR_AFTER]) + @Test // #13574: app-private storage can be accessed without storage permissions + fun `app-private collection path requires no storage permissions`() { + context.sharedPrefs().edit { + putString(CollectionHelper.PREF_COLLECTION_PATH, File(context.filesDir, "AnkiDroid").path) + } + withManageExternalStorageInManifest { + assertThat(selectStoragePermissions(context), equalTo(PermissionSet.APP_PRIVATE)) + } + } + + private val context: Context + get() = ApplicationProvider.getApplicationContext() + + /** a 'full' build: `MANAGE_EXTERNAL_STORAGE` is declared in the manifest */ + private fun withManageExternalStorageInManifest(block: () -> Unit) { + mockkObject(Permissions) + every { Permissions.canManageExternalStorage(any()) } returns true + try { + block() + } finally { + unmockkObject(Permissions) + } + } + /** * Helper for [com.ichi2.anki.selectStoragePermissions], making `currentFolderIsAccessibleAndLegacy` optional */ From 1461ef576a4ba68c78ea0bd30bdf034404732bf6 Mon Sep 17 00:00:00 2001 From: David Allison <62114487+david-allison@users.noreply.github.com> Date: Sun, 16 Aug 2026 16:18:09 +0100 Subject: [PATCH 2/2] refactor(startup): add `folder` param to getDefaultAnkiDroidDirectory For future: "Skip" on the permission screen will need the app private directory. Prep for 13574 Split from 21046 Assisted-by: Claude Fable 5 --- .../java/com/ichi2/anki/startup/SetupStorage.kt | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/startup/SetupStorage.kt b/AnkiDroid/src/main/java/com/ichi2/anki/startup/SetupStorage.kt index cf875dfd0bf7..facc56a1d97f 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/startup/SetupStorage.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/startup/SetupStorage.kt @@ -107,6 +107,8 @@ fun ensureCollectionPathSet(context: Context) { * @param directoryName The leaf folder name to use at the end of the returned path. * Defaults to `"AnkiDroid"` (the historical default-profile folder name). * Callers wanting a profile-specific layout can pass e.g. the profile id. + * @param folder The storage location to return the default directory for. + * Defaults to [selectAnkiDroidFolder]. * @return Absolute Path to the default location starting location for the AnkiDroid directory * * @throws SystemStorageException if `getExternalFilesDir` returns null @@ -116,14 +118,12 @@ fun ensureCollectionPathSet(context: Context) { fun getDefaultAnkiDroidDirectory( context: Context, directoryName: String = "AnkiDroid", -): File { - val legacyStorage = selectAnkiDroidFolder(context) != AnkiDroidFolder.APP_PRIVATE - return if (legacyStorage) { - legacyAnkiDroidDirectory(directoryName) - } else { - File(getAppSpecificExternalAnkiDroidDirectory(context), directoryName) + folder: AnkiDroidFolder = selectAnkiDroidFolder(context), +): File = + when (folder) { + AnkiDroidFolder.PUBLIC -> legacyAnkiDroidDirectory(directoryName) + AnkiDroidFolder.APP_PRIVATE -> File(getAppSpecificExternalAnkiDroidDirectory(context), directoryName) } -} /** * Returns the absolute path to the AnkiDroid directory under the primary/shared external storage directory.