Merge pull request #7809 from thunderbird/clean_up_SettingsFileParser

Refactor `SettingsImporter` [3/x]
This commit is contained in:
cketti 2024-05-07 16:19:09 +02:00 committed by GitHub
commit e9cc92078a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 129 additions and 147 deletions

View file

@ -0,0 +1,47 @@
package com.fsck.k9.preferences
import com.fsck.k9.mail.AuthType
internal typealias SettingsMap = Map<String, String>
internal interface SettingsFile {
data class Contents(
val contentVersion: Int,
val globalSettings: SettingsMap?,
val accounts: Map<String, Account>?,
)
data class Account(
val uuid: String,
val name: String?,
val incoming: Server?,
val outgoing: Server?,
val settings: SettingsMap?,
val identities: List<Identity>?,
val folders: List<Folder>?,
)
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?,
)
data class Identity(
val name: String?,
val email: String?,
val description: String?,
val settings: SettingsMap?,
)
data class Folder(
val name: String?,
val settings: SettingsMap?,
)
}

View file

@ -1,47 +0,0 @@
package com.fsck.k9.preferences
import com.fsck.k9.mail.AuthType
internal data class Imported(
val contentVersion: Int,
val globalSettings: ImportedSettings?,
val accounts: Map<String, ImportedAccount>?,
)
internal data class ImportedSettings(
val settings: Map<String, String> = emptyMap(),
)
internal data class ImportedAccount(
val uuid: String,
val name: String?,
val incoming: ImportedServer?,
val outgoing: ImportedServer?,
val settings: ImportedSettings?,
val identities: List<ImportedIdentity>?,
val folders: List<ImportedFolder>?,
)
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?,
)
internal data class ImportedIdentity(
val name: String?,
val email: String?,
val description: String?,
val settings: ImportedSettings?,
)
internal data class ImportedFolder(
val name: String?,
val settings: ImportedSettings?,
)

View file

@ -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,34 +19,9 @@ import timber.log.Timber
*/
internal class SettingsFileParser {
@Throws(SettingsImportExportException::class)
fun parseSettings(
inputStream: InputStream,
globalSettings: Boolean,
accountUuids: List<String>?,
overview: Boolean,
): Imported {
fun parseSettings(inputStream: InputStream): Contents {
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) {
@ -58,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()
@ -75,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<String, ImportedAccount>? = null
private fun readRoot(): Contents {
var generalSettings: SettingsMap? = null
var accounts: Map<String, Account>? = null
val fileFormatVersion = readFileFormatVersion()
if (fileFormatVersion != SettingsExporter.FILE_FORMAT_VERSION) {
@ -119,7 +99,7 @@ private class XmlSettingsParser(
}
}
return Imported(contentVersion, generalSettings, accounts)
return Contents(contentVersion, generalSettings, accounts)
}
private fun readFileFormatVersion(): Int {
@ -134,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<String, String>()
readElement { eventType ->
@ -161,15 +141,11 @@ private class XmlSettingsParser(
}
}
return if (settings.isNotEmpty()) {
ImportedSettings(settings)
} else {
null
}
return settings.takeIf { it.isNotEmpty() }
}
private fun readAccounts(): Map<String, ImportedAccount>? {
val accounts = mutableMapOf<String, ImportedAccount>()
private fun readAccounts(): Map<String, Account>? {
val accounts = mutableMapOf<String, Account>()
readElement { eventType ->
if (eventType == XmlPullParser.START_TAG) {
@ -195,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()
@ -203,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<ImportedIdentity>? = null
var folders: List<ImportedFolder>? = null
var incoming: Server? = null
var outgoing: Server? = null
var settings: SettingsMap? = null
var identities: List<Identity>? = null
var folders: List<Folder>? = null
readElement { eventType ->
if (eventType == XmlPullParser.START_TAG) {
@ -242,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? {
@ -258,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
@ -266,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)
@ -305,7 +281,7 @@ private class XmlSettingsParser(
}
}
return ImportedServer(
return Server(
type,
host,
port,
@ -318,8 +294,8 @@ private class XmlSettingsParser(
)
}
private fun readIdentities(): List<ImportedIdentity> {
val identities = mutableListOf<ImportedIdentity>()
private fun readIdentities(): List<Identity> {
val identities = mutableListOf<Identity>()
readElement { eventType ->
if (eventType == XmlPullParser.START_TAG) {
@ -338,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) {
@ -366,11 +342,11 @@ private class XmlSettingsParser(
}
}
return ImportedIdentity(name, email, description, settings)
return Identity(name, email, description, settings)
}
private fun readFolders(): List<ImportedFolder> {
val folders = mutableListOf<ImportedFolder>()
private fun readFolders(): List<Folder> {
val folders = mutableListOf<Folder>()
readElement { eventType ->
if (eventType == XmlPullParser.START_TAG) {
@ -391,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 {

View file

@ -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<AccountDescriptionPair>()
val erroneousAccounts = mutableListOf<AccountDescription>()
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
@ -227,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) {
@ -253,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)
@ -330,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) {
@ -387,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) {
@ -414,7 +420,7 @@ class SettingsImporter internal constructor(
editor: StorageEditor,
contentVersion: Int,
uuid: String,
account: ImportedAccount,
account: SettingsFile.Account,
) {
val accountKeyPrefix = "$uuid."
@ -457,7 +463,7 @@ class SettingsImporter internal constructor(
// Validate identity settings
val validatedSettings = IdentitySettingsDescriptions.validate(
contentVersion,
identity.settings.settings,
identity.settings,
true,
)
@ -507,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,

View file

@ -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
@ -30,15 +33,14 @@ class SettingsFileParserTest : RobolectricTest() {
</accounts>
</k9settings>
""".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)
key(accountUuid).all {
prop(ImportedAccount::uuid).isEqualTo(accountUuid)
prop(ImportedAccount::name).isEqualTo("Account")
prop(Account::uuid).isEqualTo(accountUuid)
prop(Account::name).isEqualTo("Account")
}
}
}
@ -61,16 +63,15 @@ class SettingsFileParserTest : RobolectricTest() {
</accounts>
</k9settings>
""".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)
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")
}
}
}
@ -91,16 +92,15 @@ class SettingsFileParserTest : RobolectricTest() {
</accounts>
</k9settings>
""".trimIndent().byteInputStream()
val accountUuids = listOf(accountUuid)
val results = parser.parseSettings(inputStream, true, accountUuids, false)
val results = parser.parseSettings(inputStream)
assertThat(results.accounts)
.isNotNull()
.key(accountUuid)
.prop(ImportedAccount::incoming)
.prop(Account::incoming)
.isNotNull()
.prop(ImportedServer::authenticationType)
.prop(Server::authenticationType)
.isEqualTo(AuthType.CRAM_MD5)
}
}