From 03d2946c93813602c0efd0d7d80b9d0a5852f367 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 18 Sep 2024 16:55:10 -0300 Subject: [PATCH] Remove setting for unlimited quota we set a hard limit for 1 GiB per app for now, but leave code in to make it configurable in the future --- .../seedvault/KoinInstrumentationTestApp.kt | 2 +- .../backup/KvBackupInstrumentationTest.kt | 1 - .../settings/ExpertSettingsFragment.kt | 8 ----- .../seedvault/settings/SettingsManager.kt | 3 +- .../transport/backup/BackupCoordinator.kt | 2 +- .../transport/backup/BackupModule.kt | 1 - .../seedvault/transport/backup/FullBackup.kt | 9 +----- .../seedvault/transport/backup/KVBackup.kt | 10 ------ app/src/main/res/values/strings.xml | 2 -- app/src/main/res/xml/settings_expert.xml | 6 ---- .../transport/CoordinatorIntegrationTest.kt | 3 +- .../seedvault/transport/TransportTest.kt | 1 + .../transport/backup/BackupCoordinatorTest.kt | 32 +++++-------------- .../seedvault/transport/backup/BackupTest.kt | 2 -- .../transport/backup/FullBackupTest.kt | 29 +++++++---------- .../transport/backup/KVBackupTest.kt | 1 - 16 files changed, 26 insertions(+), 86 deletions(-) diff --git a/app/src/androidTest/java/com/stevesoltys/seedvault/KoinInstrumentationTestApp.kt b/app/src/androidTest/java/com/stevesoltys/seedvault/KoinInstrumentationTestApp.kt index a55a1b7a..1d9b072c 100644 --- a/app/src/androidTest/java/com/stevesoltys/seedvault/KoinInstrumentationTestApp.kt +++ b/app/src/androidTest/java/com/stevesoltys/seedvault/KoinInstrumentationTestApp.kt @@ -37,7 +37,7 @@ class KoinInstrumentationTestApp : App() { single { spyk(BackupNotificationManager(context)) } single { spyk(FullBackup(get(), get(), get(), get())) } - single { spyk(KVBackup(get(), get(), get(), get())) } + single { spyk(KVBackup(get(), get(), get())) } single { spyk(InputFactory()) } single { spyk(FullRestore(get(), get(), get(), get(), get(), get())) } diff --git a/app/src/androidTest/java/com/stevesoltys/seedvault/transport/backup/KvBackupInstrumentationTest.kt b/app/src/androidTest/java/com/stevesoltys/seedvault/transport/backup/KvBackupInstrumentationTest.kt index 5fd73ac0..b7190753 100644 --- a/app/src/androidTest/java/com/stevesoltys/seedvault/transport/backup/KvBackupInstrumentationTest.kt +++ b/app/src/androidTest/java/com/stevesoltys/seedvault/transport/backup/KvBackupInstrumentationTest.kt @@ -40,7 +40,6 @@ class KvBackupInstrumentationTest : KoinComponent { private val dbManager: KvDbManager by inject() private val backup = KVBackup( - settingsManager = settingsManager, backupReceiver = backupReceiver, inputFactory = inputFactory, dbManager = dbManager, diff --git a/app/src/main/java/com/stevesoltys/seedvault/settings/ExpertSettingsFragment.kt b/app/src/main/java/com/stevesoltys/seedvault/settings/ExpertSettingsFragment.kt index 72c701df..0d1097ba 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/settings/ExpertSettingsFragment.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/settings/ExpertSettingsFragment.kt @@ -11,7 +11,6 @@ import androidx.activity.result.contract.ActivityResultContracts.CreateDocument import androidx.preference.Preference import androidx.preference.Preference.OnPreferenceChangeListener import androidx.preference.PreferenceFragmentCompat -import androidx.preference.SwitchPreferenceCompat import androidx.preference.TwoStatePreference import com.google.android.material.dialog.MaterialAlertDialogBuilder import com.google.android.mms.ContentType.TEXT_PLAIN @@ -65,13 +64,6 @@ class ExpertSettingsFragment : PreferenceFragmentCompat() { createFileLauncher.launch(name) true } - - val quotaPreference = findPreference(PREF_KEY_UNLIMITED_QUOTA) - - quotaPreference?.setOnPreferenceChangeListener { _, newValue -> - quotaPreference.isChecked = newValue as Boolean - true - } } override fun onStart() { diff --git a/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsManager.kt b/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsManager.kt index 6e8a41e1..0e21fae7 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsManager.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/settings/SettingsManager.kt @@ -55,7 +55,6 @@ private const val PREF_KEY_WEBDAV_PASS = "webDavPass" private const val PREF_KEY_BACKUP_APP_BLACKLIST = "backupAppBlacklist" private const val PREF_KEY_BACKUP_STORAGE = "backup_storage" -internal const val PREF_KEY_UNLIMITED_QUOTA = "unlimited_quota" internal const val PREF_KEY_LAST_BACKUP = "lastBackup" class SettingsManager(private val context: Context) { @@ -246,7 +245,7 @@ class SettingsManager(private val context: Context) { prefs.edit().putStringSet(PREF_KEY_BACKUP_APP_BLACKLIST, blacklistedApps).apply() } - fun isQuotaUnlimited() = prefs.getBoolean(PREF_KEY_UNLIMITED_QUOTA, false) + val quota: Long = 1024 * 1024 * 1024 // 1 GiB for now /** * This assumes that if there's no storage plugin set, it is the first start. diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt index a67494ce..93f66014 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt @@ -134,7 +134,7 @@ internal class BackupCoordinator( fun getBackupQuota(packageName: String, isFullBackup: Boolean): Long { // report back quota Log.i(TAG, "Get backup quota for $packageName. Is full backup: $isFullBackup.") - val quota = if (isFullBackup) full.quota else kv.quota + val quota = settingsManager.quota Log.i(TAG, "Reported quota of $quota bytes.") return quota } diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupModule.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupModule.kt index 4adc8cc1..318c7a16 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupModule.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupModule.kt @@ -21,7 +21,6 @@ val backupModule = module { single { KvDbManagerImpl(androidContext()) } single { KVBackup( - settingsManager = get(), backupReceiver = get(), inputFactory = get(), dbManager = get(), diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/FullBackup.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/FullBackup.kt index d645fe35..2156bedd 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/FullBackup.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/FullBackup.kt @@ -32,8 +32,6 @@ private class FullBackupState( var size: Long = 0 } -const val DEFAULT_QUOTA_FULL_BACKUP = (2 * (25 * 1024 * 1024)).toLong() - private val TAG = FullBackup::class.java.simpleName @Suppress("BlockingMethodInNonBlockingContext") @@ -48,12 +46,7 @@ internal class FullBackup( val hasState: Boolean get() = state != null val currentPackageInfo get() = state?.packageInfo - val quota - get() = if (settingsManager.isQuotaUnlimited()) { - Long.MAX_VALUE - } else { - DEFAULT_QUOTA_FULL_BACKUP - } + val quota get() = settingsManager.quota fun checkFullBackupSize(size: Long): Int { Log.i(TAG, "Check full backup size of $size bytes.") diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/KVBackup.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/KVBackup.kt index 6a7ad05b..18bab441 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/KVBackup.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/KVBackup.kt @@ -16,7 +16,6 @@ import android.os.ParcelFileDescriptor import android.util.Log import com.stevesoltys.seedvault.repo.BackupData import com.stevesoltys.seedvault.repo.BackupReceiver -import com.stevesoltys.seedvault.settings.SettingsManager import java.io.IOException class KVBackupState( @@ -24,12 +23,9 @@ class KVBackupState( val db: KVDb, ) -const val DEFAULT_QUOTA_KEY_VALUE_BACKUP = (2 * (5 * 1024 * 1024)).toLong() - private val TAG = KVBackup::class.java.simpleName internal class KVBackup( - private val settingsManager: SettingsManager, private val backupReceiver: BackupReceiver, private val inputFactory: InputFactory, private val dbManager: KvDbManager, @@ -39,12 +35,6 @@ internal class KVBackup( val hasState get() = state != null val currentPackageInfo get() = state?.packageInfo - val quota: Long - get() = if (settingsManager.isQuotaUnlimited()) { - Long.MAX_VALUE - } else { - DEFAULT_QUOTA_KEY_VALUE_BACKUP - } fun performBackup( packageInfo: PackageInfo, diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 1fc62f08..bbd2aa1d 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -67,8 +67,6 @@ Back up only when charging Expert settings - Unlimited app quota - Do not impose a limitation on the size of app backups.\n\nWarning: This can fill up your storage location quickly. Not needed for most apps. Save app log Developers can diagnose bugs with these logs.\n\nWarning: The log file might contain personally identifiable information. Review before and delete after sharing! Error: Could not save app log diff --git a/app/src/main/res/xml/settings_expert.xml b/app/src/main/res/xml/settings_expert.xml index cf155f90..b7c9cb4a 100644 --- a/app/src/main/res/xml/settings_expert.xml +++ b/app/src/main/res/xml/settings_expert.xml @@ -3,12 +3,6 @@ SPDX-License-Identifier: Apache-2.0 --> - - () private val backupReceiver = mockk() private val kvBackup = KVBackup( - settingsManager = settingsManager, backupReceiver = backupReceiver, inputFactory = inputFactory, dbManager = dbManager, @@ -287,7 +286,7 @@ internal class CoordinatorIntegrationTest : TransportTest() { val bInputStream = ByteArrayInputStream(appData) every { inputFactory.getInputStream(fileDescriptor) } returns bInputStream - every { settingsManager.isQuotaUnlimited() } returns false + every { settingsManager.quota } returns quota coEvery { backupReceiver.addBytes(any(), capture(byteSlot)) } answers { bOutputStream.writeBytes(byteSlot.captured) } diff --git a/app/src/test/java/com/stevesoltys/seedvault/transport/TransportTest.kt b/app/src/test/java/com/stevesoltys/seedvault/transport/TransportTest.kt index 6ebd3b60..9af4a509 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/transport/TransportTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/transport/TransportTest.kt @@ -57,6 +57,7 @@ internal abstract class TransportTest { protected val sigInfo: SigningInfo = mockk() protected val token = Random.nextLong() + protected val quota = Random.nextLong(1, Long.MAX_VALUE) protected val applicationInfo = mockk { flags = FLAG_ALLOW_BACKUP or FLAG_INSTALLED } diff --git a/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt b/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt index 7074056c..30c5cf62 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt @@ -87,11 +87,7 @@ internal class BackupCoordinatorTest : BackupTest() { val isFullBackup = Random.nextBoolean() val quota = Random.nextLong() - if (isFullBackup) { - every { full.quota } returns quota - } else { - every { kv.quota } returns quota - } + every { settingsManager.quota } returns quota assertEquals(quota, backup.getBackupQuota(packageInfo.packageName, isFullBackup)) } @@ -182,9 +178,9 @@ internal class BackupCoordinatorTest : BackupTest() { coEvery { full.performFullBackup(packageInfo, fileDescriptor, 0) } returns TRANSPORT_OK - every { full.quota } returns DEFAULT_QUOTA_FULL_BACKUP + every { settingsManager.quota } returns quota every { - full.checkFullBackupSize(DEFAULT_QUOTA_FULL_BACKUP + 1) + full.checkFullBackupSize(quota + 1) } returns TRANSPORT_QUOTA_EXCEEDED every { full.currentPackageInfo } returns packageInfo every { @@ -202,14 +198,8 @@ internal class BackupCoordinatorTest : BackupTest() { TRANSPORT_OK, backup.performFullBackup(packageInfo, fileDescriptor, 0) ) - assertEquals( - DEFAULT_QUOTA_FULL_BACKUP, - backup.getBackupQuota(packageInfo.packageName, true) - ) - assertEquals( - TRANSPORT_QUOTA_EXCEEDED, - backup.checkFullBackupSize(DEFAULT_QUOTA_FULL_BACKUP + 1) - ) + assertEquals(quota, backup.getBackupQuota(packageInfo.packageName, true)) + assertEquals(TRANSPORT_QUOTA_EXCEEDED, backup.checkFullBackupSize(quota + 1)) backup.cancelFullBackup() assertEquals(0L, backup.requestFullBackupTime()) @@ -227,7 +217,7 @@ internal class BackupCoordinatorTest : BackupTest() { coEvery { full.performFullBackup(packageInfo, fileDescriptor, 0) } returns TRANSPORT_OK - every { full.quota } returns DEFAULT_QUOTA_FULL_BACKUP + every { settingsManager.quota } returns quota every { full.checkFullBackupSize(0) } returns TRANSPORT_PACKAGE_REJECTED every { full.currentPackageInfo } returns packageInfo every { @@ -241,14 +231,8 @@ internal class BackupCoordinatorTest : BackupTest() { every { backendManager.backendProperties } returns safProperties every { settingsManager.useMeteredNetwork } returns false - assertEquals( - TRANSPORT_OK, - backup.performFullBackup(packageInfo, fileDescriptor, 0) - ) - assertEquals( - DEFAULT_QUOTA_FULL_BACKUP, - backup.getBackupQuota(packageInfo.packageName, true) - ) + assertEquals(TRANSPORT_OK, backup.performFullBackup(packageInfo, fileDescriptor, 0)) + assertEquals(quota, backup.getBackupQuota(packageInfo.packageName, true)) assertEquals(TRANSPORT_PACKAGE_REJECTED, backup.checkFullBackupSize(0)) backup.cancelFullBackup() assertEquals(0L, backup.requestFullBackupTime()) diff --git a/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupTest.kt b/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupTest.kt index f480a250..c42fd9b1 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupTest.kt @@ -17,6 +17,4 @@ internal abstract class BackupTest : TransportTest() { protected val outputStream = mockk() protected val encryptedOutputStream = mockk() - protected val quota = DEFAULT_QUOTA_FULL_BACKUP - } diff --git a/app/src/test/java/com/stevesoltys/seedvault/transport/backup/FullBackupTest.kt b/app/src/test/java/com/stevesoltys/seedvault/transport/backup/FullBackupTest.kt index 886ff74c..79fa1c41 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/transport/backup/FullBackupTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/transport/backup/FullBackupTest.kt @@ -49,21 +49,14 @@ internal class FullBackupTest : BackupTest() { @Test fun `checkFullBackupSize exceeds quota`() { - every { settingsManager.isQuotaUnlimited() } returns false + every { settingsManager.quota } returns quota assertEquals( TRANSPORT_QUOTA_EXCEEDED, - backup.checkFullBackupSize(DEFAULT_QUOTA_FULL_BACKUP + 1) + backup.checkFullBackupSize(quota + 1) ) } - @Test - fun `checkFullBackupSize does not exceed quota when unlimited`() { - every { settingsManager.isQuotaUnlimited() } returns true - - assertEquals(TRANSPORT_OK, backup.checkFullBackupSize(quota + 1)) - } - @Test fun `checkFullBackupSize for no data`() { assertEquals(TRANSPORT_PACKAGE_REJECTED, backup.checkFullBackupSize(0)) @@ -76,14 +69,14 @@ internal class FullBackupTest : BackupTest() { @Test fun `checkFullBackupSize accepts min data`() { - every { settingsManager.isQuotaUnlimited() } returns false + every { settingsManager.quota } returns quota assertEquals(TRANSPORT_OK, backup.checkFullBackupSize(1)) } @Test fun `checkFullBackupSize accepts max data`() { - every { settingsManager.isQuotaUnlimited() } returns false + every { settingsManager.quota } returns quota assertEquals(TRANSPORT_OK, backup.checkFullBackupSize(quota)) } @@ -104,7 +97,8 @@ internal class FullBackupTest : BackupTest() { @Test fun `sendBackupData first call over quota`() = runBlocking { - every { settingsManager.isQuotaUnlimited() } returns false + val quota = Random.nextInt(1, Int.MAX_VALUE).toLong() + every { settingsManager.quota } returns quota every { inputFactory.getInputStream(data) } returns inputStream val numBytes = (quota + 1).toInt() expectSendData(numBytes) @@ -123,7 +117,8 @@ internal class FullBackupTest : BackupTest() { @Test fun `sendBackupData subsequent calls over quota`() = runBlocking { - every { settingsManager.isQuotaUnlimited() } returns false + val quota = (50 * 1024 * 1024).toLong() + every { settingsManager.quota } returns quota every { inputFactory.getInputStream(data) } returns inputStream assertEquals(TRANSPORT_OK, backup.performFullBackup(packageInfo, data, 0)) @@ -155,7 +150,7 @@ internal class FullBackupTest : BackupTest() { assertEquals(TRANSPORT_OK, backup.performFullBackup(packageInfo, data, 0)) assertTrue(backup.hasState) - every { settingsManager.isQuotaUnlimited() } returns false + every { settingsManager.quota } returns quota every { inputStream.read(any(), any(), bytes.size) } throws IOException() assertEquals(TRANSPORT_ERROR, backup.sendBackupData(bytes.size)) @@ -175,7 +170,7 @@ internal class FullBackupTest : BackupTest() { assertEquals(TRANSPORT_OK, backup.performFullBackup(packageInfo, data, 0)) assertTrue(backup.hasState) - every { settingsManager.isQuotaUnlimited() } returns false + every { settingsManager.quota } returns quota every { inputStream.read(any(), 0, bytes.size) } returns bytes.size coEvery { backupReceiver.addBytes("FullBackup $packageName", any()) } throws IOException() @@ -196,7 +191,7 @@ internal class FullBackupTest : BackupTest() { assertEquals(TRANSPORT_OK, backup.performFullBackup(packageInfo, data, 0)) assertTrue(backup.hasState) - every { settingsManager.isQuotaUnlimited() } returns false + every { settingsManager.quota } returns quota expectSendData(bytes.size) assertEquals(TRANSPORT_OK, backup.sendBackupData(bytes.size)) @@ -215,7 +210,7 @@ internal class FullBackupTest : BackupTest() { @Test fun `sendBackupData runs ok`() = runBlocking { - every { settingsManager.isQuotaUnlimited() } returns false + every { settingsManager.quota } returns quota every { inputFactory.getInputStream(data) } returns inputStream assertEquals(TRANSPORT_OK, backup.performFullBackup(packageInfo, data, 0)) diff --git a/app/src/test/java/com/stevesoltys/seedvault/transport/backup/KVBackupTest.kt b/app/src/test/java/com/stevesoltys/seedvault/transport/backup/KVBackupTest.kt index 0dfb8959..209553f7 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/transport/backup/KVBackupTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/transport/backup/KVBackupTest.kt @@ -40,7 +40,6 @@ internal class KVBackupTest : BackupTest() { private val dbManager = mockk() private val backup = KVBackup( - settingsManager = settingsManager, backupReceiver = backupReceiver, inputFactory = inputFactory, dbManager = dbManager,