From c0917b251dc5036a4c266b4977026e8871e52d9b Mon Sep 17 00:00:00 2001 From: cketti Date: Fri, 3 May 2024 18:44:01 +0200 Subject: [PATCH 1/3] Move code to filter settings file contents to `SettingsImporter` --- .../fsck/k9/preferences/SettingsFileParser.kt | 29 ++---------------- .../fsck/k9/preferences/SettingsImporter.kt | 30 +++++++++++-------- .../k9/preferences/SettingsFileParserTest.kt | 9 ++---- 3 files changed, 23 insertions(+), 45 deletions(-) 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 c0bc29fc1..46bcb2f47 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 @@ -14,34 +14,9 @@ import timber.log.Timber */ internal class SettingsFileParser { @Throws(SettingsImportExportException::class) - fun parseSettings( - inputStream: InputStream, - globalSettings: Boolean, - accountUuids: List?, - overview: Boolean, - ): Imported { + fun parseSettings(inputStream: InputStream): Imported { try { - val settings = XmlSettingsParser(inputStream).parse() - - // TODO: Move this filtering code out of SettingsFileParser - val filteredGlobalSettings = if (overview) { - settings.globalSettings?.let { ImportedSettings() } - } else if (globalSettings) { - settings.globalSettings - } else { - null - } - - val filteredAccounts = if (overview || accountUuids == null) { - settings.accounts - } else { - settings.accounts?.filterKeys { it in accountUuids } - } - - return settings.copy( - globalSettings = filteredGlobalSettings, - accounts = filteredAccounts, - ) + return XmlSettingsParser(inputStream).parse() } catch (e: XmlPullParserException) { throw SettingsImportExportException("Error parsing settings XML", e) } catch (e: SettingsParserException) { 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 71a3cf1b9..092ca90cf 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 @@ -51,13 +51,7 @@ class SettingsImporter internal constructor( @Throws(SettingsImportExportException::class) fun getImportStreamContents(inputStream: InputStream): ImportContents { try { - // Parse the import stream but don't save individual settings (overview=true) - val imported = settingsFileParser.parseSettings( - inputStream = inputStream, - globalSettings = false, - accountUuids = null, - overview = true, - ) + val imported = settingsFileParser.parseSettings(inputStream) // If the stream contains global settings the "globalSettings" member will not be null val globalSettings = (imported.globalSettings != null) @@ -101,11 +95,23 @@ class SettingsImporter internal constructor( val importedAccounts = mutableListOf() val erroneousAccounts = mutableListOf() - val imported = settingsFileParser.parseSettings( - inputStream = inputStream, - globalSettings = globalSettings, - accountUuids = accountUuids, - overview = false, + val settings = settingsFileParser.parseSettings(inputStream) + + val filteredGlobalSettings = if (globalSettings) { + settings.globalSettings + } else { + null + } + + val filteredAccounts = if (accountUuids == null) { + settings.accounts + } else { + settings.accounts?.filterKeys { it in accountUuids } + } + + val imported = settings.copy( + globalSettings = filteredGlobalSettings, + accounts = filteredAccounts, ) val storage = preferences.storage 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 be9466ec7..98e67a8fe 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 @@ -30,9 +30,8 @@ class SettingsFileParserTest : RobolectricTest() { """.trimIndent().byteInputStream() - val accountUuids = listOf("1") - val results = parser.parseSettings(inputStream, true, accountUuids, true) + val results = parser.parseSettings(inputStream) assertThat(results.accounts).isNotNull().all { hasSize(1) @@ -61,9 +60,8 @@ class SettingsFileParserTest : RobolectricTest() { """.trimIndent().byteInputStream() - val accountUuids = listOf("1") - val results = parser.parseSettings(inputStream, true, accountUuids, true) + val results = parser.parseSettings(inputStream) assertThat(results.accounts).isNotNull().all { hasSize(1) @@ -91,9 +89,8 @@ class SettingsFileParserTest : RobolectricTest() { """.trimIndent().byteInputStream() - val accountUuids = listOf(accountUuid) - val results = parser.parseSettings(inputStream, true, accountUuids, false) + val results = parser.parseSettings(inputStream) assertThat(results.accounts) .isNotNull() From f23a1063f7e84b66d5abfc089af0de6402a393c3 Mon Sep 17 00:00:00 2001 From: cketti Date: Fri, 3 May 2024 19:15:43 +0200 Subject: [PATCH 2/3] Rename `SettingsFileContents.kt` to `SettingsFile.kt` --- .../k9/preferences/{SettingsFileContents.kt => SettingsFile.kt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename app/core/src/main/java/com/fsck/k9/preferences/{SettingsFileContents.kt => SettingsFile.kt} (100%) diff --git a/app/core/src/main/java/com/fsck/k9/preferences/SettingsFileContents.kt b/app/core/src/main/java/com/fsck/k9/preferences/SettingsFile.kt similarity index 100% rename from app/core/src/main/java/com/fsck/k9/preferences/SettingsFileContents.kt rename to app/core/src/main/java/com/fsck/k9/preferences/SettingsFile.kt From d618417c3f5f89cfafcc4a48f6d1ca16ea8a8efb Mon Sep 17 00:00:00 2001 From: cketti Date: Fri, 3 May 2024 19:16:44 +0200 Subject: [PATCH 3/3] Rename data classes returned by `SettingsFileParser` --- .../com/fsck/k9/preferences/SettingsFile.kt | 76 +++++++++--------- .../fsck/k9/preferences/SettingsFileParser.kt | 77 ++++++++++--------- .../fsck/k9/preferences/SettingsImporter.kt | 22 +++--- .../k9/preferences/SettingsFileParserTest.kt | 17 ++-- 4 files changed, 98 insertions(+), 94 deletions(-) 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 ae69d5711..91c512653 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 @@ -2,46 +2,46 @@ package com.fsck.k9.preferences import com.fsck.k9.mail.AuthType -internal data class Imported( - val contentVersion: Int, - val globalSettings: ImportedSettings?, - val accounts: Map?, -) +internal typealias SettingsMap = Map -internal data class ImportedSettings( - val settings: Map = emptyMap(), -) +internal interface SettingsFile { + data class Contents( + val contentVersion: Int, + val globalSettings: SettingsMap?, + val accounts: Map?, + ) -internal data class ImportedAccount( - val uuid: String, - val name: String?, - val incoming: ImportedServer?, - val outgoing: ImportedServer?, - val settings: ImportedSettings?, - val identities: List?, - val folders: List?, -) + data class Account( + val uuid: String, + val name: String?, + val incoming: Server?, + val outgoing: Server?, + val settings: SettingsMap?, + val identities: List?, + val folders: List?, + ) -internal data class ImportedServer( - val type: String?, - val host: String?, - val port: String?, - val connectionSecurity: String?, - val authenticationType: AuthType?, - val username: String?, - val password: String?, - val clientCertificateAlias: String?, - val extras: ImportedSettings?, -) + data class Server( + val type: String?, + val host: String?, + val port: String?, + val connectionSecurity: String?, + val authenticationType: AuthType?, + val username: String?, + val password: String?, + val clientCertificateAlias: String?, + val extras: SettingsMap?, + ) -internal data class ImportedIdentity( - val name: String?, - val email: String?, - val description: String?, - val settings: ImportedSettings?, -) + data class Identity( + val name: String?, + val email: String?, + val description: String?, + val settings: SettingsMap?, + ) -internal data class ImportedFolder( - val name: String?, - val settings: ImportedSettings?, -) + data class Folder( + val name: String?, + val settings: SettingsMap?, + ) +} 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 46bcb2f47..cb9aa260c 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 @@ -1,6 +1,11 @@ package com.fsck.k9.preferences import com.fsck.k9.mail.AuthType +import com.fsck.k9.preferences.SettingsFile.Account +import com.fsck.k9.preferences.SettingsFile.Contents +import com.fsck.k9.preferences.SettingsFile.Folder +import com.fsck.k9.preferences.SettingsFile.Identity +import com.fsck.k9.preferences.SettingsFile.Server import java.io.InputStream import java.io.InputStreamReader import java.util.UUID @@ -14,7 +19,7 @@ import timber.log.Timber */ internal class SettingsFileParser { @Throws(SettingsImportExportException::class) - fun parseSettings(inputStream: InputStream): Imported { + fun parseSettings(inputStream: InputStream): Contents { try { return XmlSettingsParser(inputStream).parse() } catch (e: XmlPullParserException) { @@ -33,15 +38,15 @@ private class XmlSettingsParser( setInput(InputStreamReader(inputStream)) } - fun parse(): Imported { - var imported: Imported? = null + fun parse(): Contents { + var contents: Contents? = null do { val eventType = pullParser.next() if (eventType == XmlPullParser.START_TAG) { when (pullParser.name) { SettingsExporter.ROOT_ELEMENT -> { - imported = readRoot() + contents = readRoot() } else -> { skipElement() @@ -50,16 +55,16 @@ private class XmlSettingsParser( } } while (eventType != XmlPullParser.END_DOCUMENT) - if (imported == null) { + if (contents == null) { parserError("Missing '${SettingsExporter.ROOT_ELEMENT}' element") } - return imported + return contents } - private fun readRoot(): Imported { - var generalSettings: ImportedSettings? = null - var accounts: Map? = null + private fun readRoot(): Contents { + var generalSettings: SettingsMap? = null + var accounts: Map? = null val fileFormatVersion = readFileFormatVersion() if (fileFormatVersion != SettingsExporter.FILE_FORMAT_VERSION) { @@ -94,7 +99,7 @@ private class XmlSettingsParser( } } - return Imported(contentVersion, generalSettings, accounts) + return Contents(contentVersion, generalSettings, accounts) } private fun readFileFormatVersion(): Int { @@ -109,11 +114,11 @@ private class XmlSettingsParser( ?: parserError("Invalid content version: $versionString") } - private fun readGlobalSettings(): ImportedSettings? { + private fun readGlobalSettings(): SettingsMap? { return readSettingsContainer() } - private fun readSettingsContainer(): ImportedSettings? { + private fun readSettingsContainer(): SettingsMap? { val settings = mutableMapOf() readElement { eventType -> @@ -136,15 +141,11 @@ private class XmlSettingsParser( } } - return if (settings.isNotEmpty()) { - ImportedSettings(settings) - } else { - null - } + return settings.takeIf { it.isNotEmpty() } } - private fun readAccounts(): Map? { - val accounts = mutableMapOf() + private fun readAccounts(): Map? { + val accounts = mutableMapOf() readElement { eventType -> if (eventType == XmlPullParser.START_TAG) { @@ -170,7 +171,7 @@ private class XmlSettingsParser( return accounts.takeIf { it.isNotEmpty() } } - private fun readAccount(): ImportedAccount? { + private fun readAccount(): Account? { val uuid = readUuid() if (uuid == null) { skipElement() @@ -178,11 +179,11 @@ private class XmlSettingsParser( } var name: String? = null - var incoming: ImportedServer? = null - var outgoing: ImportedServer? = null - var settings: ImportedSettings? = null - var identities: List? = null - var folders: List? = null + var incoming: Server? = null + var outgoing: Server? = null + var settings: SettingsMap? = null + var identities: List? = null + var folders: List? = null readElement { eventType -> if (eventType == XmlPullParser.START_TAG) { @@ -217,7 +218,7 @@ private class XmlSettingsParser( name = uuid } - return ImportedAccount(uuid, name, incoming, outgoing, settings, identities, folders) + return Account(uuid, name, incoming, outgoing, settings, identities, folders) } private fun readUuid(): String? { @@ -233,7 +234,7 @@ private class XmlSettingsParser( return uuid } - private fun readServerSettings(): ImportedServer { + private fun readServerSettings(): Server { var host: String? = null var port: String? = null var connectionSecurity: String? = null @@ -241,7 +242,7 @@ private class XmlSettingsParser( var username: String? = null var password: String? = null var clientCertificateAlias: String? = null - var extras: ImportedSettings? = null + var extras: SettingsMap? = null val type = pullParser.getAttributeValue(null, SettingsExporter.TYPE_ATTRIBUTE) @@ -280,7 +281,7 @@ private class XmlSettingsParser( } } - return ImportedServer( + return Server( type, host, port, @@ -293,8 +294,8 @@ private class XmlSettingsParser( ) } - private fun readIdentities(): List { - val identities = mutableListOf() + private fun readIdentities(): List { + val identities = mutableListOf() readElement { eventType -> if (eventType == XmlPullParser.START_TAG) { @@ -313,11 +314,11 @@ private class XmlSettingsParser( return identities } - private fun readIdentity(): ImportedIdentity { + private fun readIdentity(): Identity { var name: String? = null var email: String? = null var description: String? = null - var settings: ImportedSettings? = null + var settings: SettingsMap? = null readElement { eventType -> if (eventType == XmlPullParser.START_TAG) { @@ -341,11 +342,11 @@ private class XmlSettingsParser( } } - return ImportedIdentity(name, email, description, settings) + return Identity(name, email, description, settings) } - private fun readFolders(): List { - val folders = mutableListOf() + private fun readFolders(): List { + val folders = mutableListOf() readElement { eventType -> if (eventType == XmlPullParser.START_TAG) { @@ -366,11 +367,11 @@ private class XmlSettingsParser( return folders } - private fun readFolder(): ImportedFolder? { + private fun readFolder(): Folder? { val name = pullParser.getAttributeValue(null, SettingsExporter.NAME_ATTRIBUTE) ?: return null val settings = readSettingsContainer() - return ImportedFolder(name, settings) + return Folder(name, settings) } private fun readText(): String { 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 092ca90cf..9081d830d 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 @@ -233,10 +233,10 @@ class SettingsImporter internal constructor( storage: Storage, editor: StorageEditor, contentVersion: Int, - settings: ImportedSettings, + settings: SettingsMap, ) { // Validate global settings - val validatedSettings = GeneralSettingsDescriptions.validate(contentVersion, settings.settings) + val validatedSettings = GeneralSettingsDescriptions.validate(contentVersion, settings) // Upgrade global settings to current content version if (contentVersion != Settings.VERSION) { @@ -259,7 +259,7 @@ class SettingsImporter internal constructor( private fun importAccount( editor: StorageEditor, contentVersion: Int, - account: ImportedAccount, + account: SettingsFile.Account, ): AccountDescriptionPair { val original = AccountDescription(account.name!!, account.uuid) @@ -336,7 +336,7 @@ class SettingsImporter internal constructor( // Validate account settings val validatedSettings = - AccountSettingsDescriptions.validate(contentVersion, account.settings!!.settings, true) + AccountSettingsDescriptions.validate(contentVersion, account.settings!!, true) // Upgrade account settings to current content version if (contentVersion != Settings.VERSION) { @@ -393,11 +393,11 @@ class SettingsImporter internal constructor( editor: StorageEditor, contentVersion: Int, uuid: String, - folder: ImportedFolder, + folder: SettingsFile.Folder, ) { // Validate folder settings val validatedSettings = - FolderSettingsDescriptions.validate(contentVersion, folder.settings!!.settings, true) + FolderSettingsDescriptions.validate(contentVersion, folder.settings!!, true) // Upgrade folder settings to current content version if (contentVersion != Settings.VERSION) { @@ -420,7 +420,7 @@ class SettingsImporter internal constructor( editor: StorageEditor, contentVersion: Int, uuid: String, - account: ImportedAccount, + account: SettingsFile.Account, ) { val accountKeyPrefix = "$uuid." @@ -463,7 +463,7 @@ class SettingsImporter internal constructor( // Validate identity settings val validatedSettings = IdentitySettingsDescriptions.validate( contentVersion, - identity.settings.settings, + identity.settings, true, ) @@ -513,18 +513,18 @@ class SettingsImporter internal constructor( editor.putString(key, value) } - private fun getAccountDisplayName(account: ImportedAccount): String { + private fun getAccountDisplayName(account: SettingsFile.Account): String { return account.name?.takeIf { it.isNotEmpty() } ?: account.identities?.firstOrNull()?.email ?: error("Account name missing") } - private fun createServerSettings(importedServer: ImportedServer): ServerSettings { + private fun createServerSettings(importedServer: SettingsFile.Server): ServerSettings { val type = toServerSettingsType(importedServer.type!!) val port = convertPort(importedServer.port) val connectionSecurity = convertConnectionSecurity(importedServer.connectionSecurity) val password = if (importedServer.authenticationType == AuthType.XOAUTH2) "" else importedServer.password - val extra = importedServer.extras?.settings.orEmpty() + val extra = importedServer.extras.orEmpty() return ServerSettings( type, 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 98e67a8fe..688651e94 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 @@ -11,6 +11,9 @@ 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 +import com.fsck.k9.preferences.SettingsFile.Identity +import com.fsck.k9.preferences.SettingsFile.Server import java.util.UUID import org.junit.Test @@ -36,8 +39,8 @@ class SettingsFileParserTest : RobolectricTest() { assertThat(results.accounts).isNotNull().all { hasSize(1) key(accountUuid).all { - prop(ImportedAccount::uuid).isEqualTo(accountUuid) - prop(ImportedAccount::name).isEqualTo("Account") + prop(Account::uuid).isEqualTo(accountUuid) + prop(Account::name).isEqualTo("Account") } } } @@ -66,9 +69,9 @@ class SettingsFileParserTest : RobolectricTest() { assertThat(results.accounts).isNotNull().all { hasSize(1) key(accountUuid).all { - prop(ImportedAccount::uuid).isEqualTo(accountUuid) - prop(ImportedAccount::identities).isNotNull() - .extracting(ImportedIdentity::email).containsExactly("user@gmail.com") + prop(Account::uuid).isEqualTo(accountUuid) + prop(Account::identities).isNotNull() + .extracting(Identity::email).containsExactly("user@gmail.com") } } } @@ -95,9 +98,9 @@ class SettingsFileParserTest : RobolectricTest() { assertThat(results.accounts) .isNotNull() .key(accountUuid) - .prop(ImportedAccount::incoming) + .prop(Account::incoming) .isNotNull() - .prop(ImportedServer::authenticationType) + .prop(Server::authenticationType) .isEqualTo(AuthType.CRAM_MD5) } }