diff --git a/app/core/src/main/java/com/fsck/k9/preferences/SettingsFile.kt b/app/core/src/main/java/com/fsck/k9/preferences/SettingsFile.kt index 91c51265357..7c5e29b5cbe 100644 --- a/app/core/src/main/java/com/fsck/k9/preferences/SettingsFile.kt +++ b/app/core/src/main/java/com/fsck/k9/preferences/SettingsFile.kt @@ -8,7 +8,7 @@ internal interface SettingsFile { data class Contents( val contentVersion: Int, val globalSettings: SettingsMap?, - val accounts: Map?, + val accounts: List, ) data class Account( diff --git a/app/core/src/main/java/com/fsck/k9/preferences/SettingsFileParser.kt b/app/core/src/main/java/com/fsck/k9/preferences/SettingsFileParser.kt index cb9aa260c94..982ee3d2194 100644 --- a/app/core/src/main/java/com/fsck/k9/preferences/SettingsFileParser.kt +++ b/app/core/src/main/java/com/fsck/k9/preferences/SettingsFileParser.kt @@ -64,7 +64,7 @@ private class XmlSettingsParser( private fun readRoot(): Contents { var generalSettings: SettingsMap? = null - var accounts: Map? = null + var accounts: List? = null val fileFormatVersion = readFileFormatVersion() if (fileFormatVersion != SettingsExporter.FILE_FORMAT_VERSION) { @@ -99,7 +99,7 @@ private class XmlSettingsParser( } } - return Contents(contentVersion, generalSettings, accounts) + return Contents(contentVersion, generalSettings, accounts.orEmpty()) } private fun readFileFormatVersion(): Int { @@ -144,8 +144,8 @@ private class XmlSettingsParser( return settings.takeIf { it.isNotEmpty() } } - private fun readAccounts(): Map? { - val accounts = mutableMapOf() + private fun readAccounts(): List { + val accounts = mutableListOf() readElement { eventType -> if (eventType == XmlPullParser.START_TAG) { @@ -155,10 +155,10 @@ private class XmlSettingsParser( if (account == null) { // Do nothing - readAccount() already logged a message - } else if (!accounts.containsKey(account.uuid)) { - accounts[account.uuid] = account - } else { + } else if (accounts.any { it.uuid == account.uuid }) { Timber.w("Duplicate account entries with UUID %s. Ignoring!", account.uuid) + } else { + accounts.add(account) } } else -> { @@ -168,7 +168,7 @@ private class XmlSettingsParser( } } - return accounts.takeIf { it.isNotEmpty() } + return accounts } private fun readAccount(): Account? { diff --git a/app/core/src/main/java/com/fsck/k9/preferences/SettingsImporter.kt b/app/core/src/main/java/com/fsck/k9/preferences/SettingsImporter.kt index 9081d830db9..d98505efd7f 100644 --- a/app/core/src/main/java/com/fsck/k9/preferences/SettingsImporter.kt +++ b/app/core/src/main/java/com/fsck/k9/preferences/SettingsImporter.kt @@ -56,7 +56,7 @@ class SettingsImporter internal constructor( // If the stream contains global settings the "globalSettings" member will not be null val globalSettings = (imported.globalSettings != null) - val accounts = imported.accounts?.values.orEmpty().map { importedAccount -> + val accounts = imported.accounts.map { importedAccount -> AccountDescription( name = getAccountDisplayName(importedAccount), uuid = importedAccount.uuid, @@ -88,7 +88,7 @@ class SettingsImporter internal constructor( fun importSettings( inputStream: InputStream, globalSettings: Boolean, - accountUuids: List?, + accountUuids: List, ): ImportResults { try { var globalSettingsImported = false @@ -103,11 +103,7 @@ class SettingsImporter internal constructor( null } - val filteredAccounts = if (accountUuids == null) { - settings.accounts - } else { - settings.accounts?.filterKeys { it in accountUuids } - } + val filteredAccounts = settings.accounts.filter { it.uuid in accountUuids } val imported = settings.copy( globalSettings = filteredGlobalSettings, @@ -136,75 +132,71 @@ class SettingsImporter internal constructor( } } - if (!accountUuids.isNullOrEmpty()) { - if (imported.accounts != null) { - for (accountUuid in accountUuids) { - val account = imported.accounts[accountUuid] - if (account != null) { - try { - var editor = preferences.createStorageEditor() - - val importResult = importAccount(editor, imported.contentVersion, account) - - if (editor.commit()) { - Timber.v( - "Committed settings for account \"%s\" to the settings database.", - importResult.imported.name, - ) - - // Add UUID of the account we just imported to the list of account UUIDs - editor = preferences.createStorageEditor() - - val newUuid = importResult.imported.uuid - val oldAccountUuids = preferences.storage.getString("accountUuids", "") - val newAccountUuids = if (oldAccountUuids.isNotEmpty()) { - "$oldAccountUuids,$newUuid" - } else { - newUuid - } - - putString(editor, "accountUuids", newAccountUuids) - - if (!editor.commit()) { - throw SettingsImportExportException("Failed to set account UUID list") - } - - // Reload accounts - preferences.loadAccounts() - - importedAccounts.add(importResult) - } else { - Timber.w( - "Error while committing settings for account \"%s\" to the settings database.", - importResult.original.name, - ) - - erroneousAccounts.add(importResult.original) - } - } catch (e: InvalidSettingValueException) { - Timber.e(e, "Encountered invalid setting while importing account \"%s\"", account.name) - - erroneousAccounts.add(AccountDescription(account.name!!, account.uuid)) - } catch (e: Exception) { - Timber.e(e, "Exception while importing account \"%s\"", account.name) - - erroneousAccounts.add(AccountDescription(account.name!!, account.uuid)) + if (accountUuids.isNotEmpty()) { + val foundAccountUuids = imported.accounts.map { it.uuid }.toSet() + val missingAccountUuids = accountUuids.toSet() - foundAccountUuids + if (missingAccountUuids.isNotEmpty()) { + for (accountUuid in missingAccountUuids) { + Timber.w("Was asked to import account %s. But this account wasn't found.", accountUuid) + } + } + + for (account in imported.accounts) { + try { + var editor = preferences.createStorageEditor() + + val importResult = importAccount(editor, imported.contentVersion, account) + + if (editor.commit()) { + Timber.v( + "Committed settings for account \"%s\" to the settings database.", + importResult.imported.name, + ) + + // Add UUID of the account we just imported to the list of account UUIDs + editor = preferences.createStorageEditor() + + val newUuid = importResult.imported.uuid + val oldAccountUuids = preferences.storage.getString("accountUuids", "") + val newAccountUuids = if (oldAccountUuids.isNotEmpty()) { + "$oldAccountUuids,$newUuid" + } else { + newUuid } + + putString(editor, "accountUuids", newAccountUuids) + + if (!editor.commit()) { + throw SettingsImportExportException("Failed to set account UUID list") + } + + // Reload accounts + preferences.loadAccounts() + + importedAccounts.add(importResult) } else { Timber.w( - "Was asked to import account with UUID %s. But this account wasn't found.", - accountUuid, + "Error while committing settings for account \"%s\" to the settings database.", + importResult.original.name, ) + + erroneousAccounts.add(importResult.original) } - } + } catch (e: InvalidSettingValueException) { + Timber.e(e, "Encountered invalid setting while importing account \"%s\"", account.name) - val editor = preferences.createStorageEditor() + erroneousAccounts.add(AccountDescription(account.name!!, account.uuid)) + } catch (e: Exception) { + Timber.e(e, "Exception while importing account \"%s\"", account.name) - if (!editor.commit()) { - throw SettingsImportExportException("Failed to set default account") + erroneousAccounts.add(AccountDescription(account.name!!, account.uuid)) } - } else { - Timber.w("Was asked to import at least one account but none found.") + } + + val editor = preferences.createStorageEditor() + + if (!editor.commit()) { + throw SettingsImportExportException("Failed to set default account") } } diff --git a/app/core/src/test/java/com/fsck/k9/preferences/SettingsFileParserTest.kt b/app/core/src/test/java/com/fsck/k9/preferences/SettingsFileParserTest.kt index 688651e94ec..01a3aa9c098 100644 --- a/app/core/src/test/java/com/fsck/k9/preferences/SettingsFileParserTest.kt +++ b/app/core/src/test/java/com/fsck/k9/preferences/SettingsFileParserTest.kt @@ -6,9 +6,9 @@ import assertk.assertThat import assertk.assertions.containsExactly import assertk.assertions.extracting import assertk.assertions.hasSize +import assertk.assertions.index import assertk.assertions.isEqualTo import assertk.assertions.isNotNull -import assertk.assertions.key import assertk.assertions.prop import com.fsck.k9.mail.AuthType import com.fsck.k9.preferences.SettingsFile.Account @@ -38,7 +38,7 @@ class SettingsFileParserTest : RobolectricTest() { assertThat(results.accounts).isNotNull().all { hasSize(1) - key(accountUuid).all { + index(0).all { prop(Account::uuid).isEqualTo(accountUuid) prop(Account::name).isEqualTo("Account") } @@ -68,7 +68,7 @@ class SettingsFileParserTest : RobolectricTest() { assertThat(results.accounts).isNotNull().all { hasSize(1) - key(accountUuid).all { + index(0).all { prop(Account::uuid).isEqualTo(accountUuid) prop(Account::identities).isNotNull() .extracting(Identity::email).containsExactly("user@gmail.com") @@ -97,7 +97,7 @@ class SettingsFileParserTest : RobolectricTest() { assertThat(results.accounts) .isNotNull() - .key(accountUuid) + .index(0) .prop(Account::incoming) .isNotNull() .prop(Server::authenticationType)