diff --git a/app-feature-preview/src/main/java/app/k9mail/feature/preview/FeatureModule.kt b/app-feature-preview/src/main/java/app/k9mail/feature/preview/FeatureModule.kt index a2b003825c389b172ce2c1a58bd7954414ec6b3c..aa80e7390e6126dafc3c7eeab84859c7782fd63d 100644 --- a/app-feature-preview/src/main/java/app/k9mail/feature/preview/FeatureModule.kt +++ b/app-feature-preview/src/main/java/app/k9mail/feature/preview/FeatureModule.kt @@ -27,7 +27,7 @@ val accountModule: Module = module { arrayOf( AccountCommonExternalContract.AccountStateLoader::class, AccountSetupExternalContract.AccountCreator::class, - AccountEditExternalContract.AccountUpdater::class, + AccountEditExternalContract.AccountServerSettingsUpdater::class, ), ) factory { AccountOwnerNameProvider() } diff --git a/app-feature-preview/src/main/java/app/k9mail/feature/preview/account/InMemoryAccountStore.kt b/app-feature-preview/src/main/java/app/k9mail/feature/preview/account/InMemoryAccountStore.kt index fdb4136509985ef6059e3bacc23e6d0ba3001232..f67e89c88f6bb41bf18d8e31a2daadb3fd9909a0 100644 --- a/app-feature-preview/src/main/java/app/k9mail/feature/preview/account/InMemoryAccountStore.kt +++ b/app-feature-preview/src/main/java/app/k9mail/feature/preview/account/InMemoryAccountStore.kt @@ -4,14 +4,16 @@ import app.k9mail.feature.account.common.AccountCommonExternalContract.AccountSt import app.k9mail.feature.account.common.domain.entity.Account import app.k9mail.feature.account.common.domain.entity.AccountState import app.k9mail.feature.account.common.domain.entity.AuthorizationState -import app.k9mail.feature.account.edit.AccountEditExternalContract.AccountUpdater -import app.k9mail.feature.account.edit.AccountEditExternalContract.AccountUpdater.AccountUpdaterResult +import app.k9mail.feature.account.edit.AccountEditExternalContract.AccountServerSettingsUpdater +import app.k9mail.feature.account.edit.AccountEditExternalContract.AccountUpdaterFailure +import app.k9mail.feature.account.edit.AccountEditExternalContract.AccountUpdaterResult import app.k9mail.feature.account.setup.AccountSetupExternalContract.AccountCreator import app.k9mail.feature.account.setup.AccountSetupExternalContract.AccountCreator.AccountCreatorResult +import com.fsck.k9.mail.ServerSettings class InMemoryAccountStore( private val accountMap: MutableMap = mutableMapOf(), -) : AccountCreator, AccountUpdater, AccountStateLoader { +) : AccountCreator, AccountServerSettingsUpdater, AccountStateLoader { suspend fun load(accountUuid: String): Account? { return accountMap[accountUuid] @@ -27,11 +29,21 @@ class InMemoryAccountStore( return AccountCreatorResult.Success(account.uuid) } - override suspend fun updateAccount(account: Account): AccountUpdaterResult { - return if (!accountMap.containsKey(account.uuid)) { - AccountUpdaterResult.Error("Account not found") + override suspend fun updateServerSettings( + accountUuid: String, + isIncoming: Boolean, + serverSettings: ServerSettings, + ): AccountUpdaterResult { + return if (!accountMap.containsKey(accountUuid)) { + AccountUpdaterResult.Failure(AccountUpdaterFailure.AccountNotFound(accountUuid)) } else { - accountMap[account.uuid] = account + val account = accountMap[accountUuid]!! + + accountMap[account.uuid] = if (isIncoming) { + account.copy(incomingServerSettings = serverSettings) + } else { + account.copy(outgoingServerSettings = serverSettings) + } AccountUpdaterResult.Success(account.uuid) } diff --git a/app/k9mail/src/main/java/com/fsck/k9/account/AccountModule.kt b/app/k9mail/src/main/java/com/fsck/k9/account/AccountModule.kt index 677c544e7e05e3eeb58b04bca9847af14aefad78..793405215f5fcd5aba40aa3ae1e68892ddbde327 100644 --- a/app/k9mail/src/main/java/com/fsck/k9/account/AccountModule.kt +++ b/app/k9mail/src/main/java/com/fsck/k9/account/AccountModule.kt @@ -28,9 +28,9 @@ val newAccountModule = module { ) } - factory { - AccountUpdater( - preferences = get(), + factory { + AccountServerSettingsUpdater( + accountManager = get(), ) } } diff --git a/app/k9mail/src/main/java/com/fsck/k9/account/AccountServerSettingsUpdater.kt b/app/k9mail/src/main/java/com/fsck/k9/account/AccountServerSettingsUpdater.kt new file mode 100644 index 0000000000000000000000000000000000000000..f91f30354c27b72dc025080fb4a81a5c8c2b5bae --- /dev/null +++ b/app/k9mail/src/main/java/com/fsck/k9/account/AccountServerSettingsUpdater.kt @@ -0,0 +1,54 @@ +package com.fsck.k9.account + +import app.k9mail.feature.account.edit.AccountEditExternalContract +import app.k9mail.feature.account.edit.AccountEditExternalContract.AccountUpdaterFailure +import app.k9mail.feature.account.edit.AccountEditExternalContract.AccountUpdaterResult +import com.fsck.k9.logging.Timber +import com.fsck.k9.mail.ServerSettings +import com.fsck.k9.preferences.AccountManager +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext + +class AccountServerSettingsUpdater( + private val accountManager: AccountManager, + private val coroutineDispatcher: CoroutineDispatcher = Dispatchers.IO, +) : AccountEditExternalContract.AccountServerSettingsUpdater { + + @Suppress("TooGenericExceptionCaught") + override suspend fun updateServerSettings( + accountUuid: String, + isIncoming: Boolean, + serverSettings: ServerSettings, + ): AccountUpdaterResult { + return try { + withContext(coroutineDispatcher) { + updateSettings(accountUuid, isIncoming, serverSettings) + } + } catch (error: Exception) { + Timber.e(error, "Error while updating account server settings with UUID %s", accountUuid) + + AccountUpdaterResult.Failure(AccountUpdaterFailure.UnknownError(error)) + } + } + + private fun updateSettings( + accountUuid: String, + isIncoming: Boolean, + serverSettings: ServerSettings, + ): AccountUpdaterResult { + val account = accountManager.getAccount(accountUuid = accountUuid) ?: return AccountUpdaterResult.Failure( + AccountUpdaterFailure.AccountNotFound(accountUuid), + ) + + if (isIncoming) { + account.incomingServerSettings = serverSettings + } else { + account.outgoingServerSettings = serverSettings + } + + accountManager.saveAccount(account) + + return AccountUpdaterResult.Success(accountUuid) + } +} diff --git a/app/k9mail/src/main/java/com/fsck/k9/account/AccountUpdater.kt b/app/k9mail/src/main/java/com/fsck/k9/account/AccountUpdater.kt deleted file mode 100644 index 3a5bd414317b5b42bd9a3fefbaa4b20081439f65..0000000000000000000000000000000000000000 --- a/app/k9mail/src/main/java/com/fsck/k9/account/AccountUpdater.kt +++ /dev/null @@ -1,44 +0,0 @@ -package com.fsck.k9.account - -import app.k9mail.feature.account.common.domain.entity.Account -import app.k9mail.feature.account.edit.AccountEditExternalContract -import app.k9mail.feature.account.edit.AccountEditExternalContract.AccountUpdater.AccountUpdaterResult -import com.fsck.k9.Preferences -import com.fsck.k9.logging.Timber -import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.withContext - -class AccountUpdater( - private val preferences: Preferences, - private val coroutineDispatcher: CoroutineDispatcher = Dispatchers.IO, -) : AccountEditExternalContract.AccountUpdater { - - @Suppress("TooGenericExceptionCaught") - override suspend fun updateAccount(account: Account): AccountUpdaterResult { - return try { - withContext(coroutineDispatcher) { - AccountUpdaterResult.Success(update(account)) - } - } catch (e: Exception) { - Timber.e(e, "Error while updating account") - - AccountUpdaterResult.Error(e.message ?: "Unknown update account error") - } - } - - private fun update(account: Account): String { - val uuid = account.uuid - require(uuid.isNotEmpty()) { "Can't update account without uuid" } - - val existingAccount = preferences.getAccount(uuid) - require(existingAccount != null) { "Can't update non-existing account" } - - existingAccount.incomingServerSettings = account.incomingServerSettings - existingAccount.outgoingServerSettings = account.outgoingServerSettings - - preferences.saveAccount(existingAccount) - - return uuid - } -} diff --git a/app/k9mail/src/test/java/com/fsck/k9/account/AccountStateLoaderTest.kt b/app/k9mail/src/test/java/com/fsck/k9/account/AccountStateLoaderTest.kt index be7b5f940271c39f57779bc1c8e5c9cb6fad3fe6..234b62a701603ab4d366a4ec14ef01383bb19f56 100644 --- a/app/k9mail/src/test/java/com/fsck/k9/account/AccountStateLoaderTest.kt +++ b/app/k9mail/src/test/java/com/fsck/k9/account/AccountStateLoaderTest.kt @@ -16,18 +16,18 @@ import org.junit.Test class AccountStateLoaderTest { @Test - fun `loadAccountState() should return null when accountManager returns null`() = runTest { + fun `loadAccountState() SHOULD return null when accountManager returns null`() = runTest { val accountManager = FakeAccountManager() - val accountLoader = AccountStateLoader(accountManager) + val testSubject = AccountStateLoader(accountManager) - val result = accountLoader.loadAccountState("accountUuid") + val result = testSubject.loadAccountState("accountUuid") assertThat(result).isNull() } @Test - fun `loadAccountState() should return account when present in accountManager`() = runTest { - val accounts = mapOf( + fun `loadAccountState() SHOULD return account when present in accountManager`() = runTest { + val accounts = mutableMapOf( "accountUuid" to Account(uuid = "accountUuid").apply { identities = mutableListOf(Identity()) email = "emailAddress" @@ -37,9 +37,9 @@ class AccountStateLoaderTest { }, ) val accountManager = FakeAccountManager(accounts = accounts) - val accountLoader = AccountStateLoader(accountManager) + val testSubject = AccountStateLoader(accountManager) - val result = accountLoader.loadAccountState("accountUuid") + val result = testSubject.loadAccountState("accountUuid") assertThat(result).isEqualTo( AccountState( diff --git a/app/k9mail/src/test/java/com/fsck/k9/account/AccountUpdaterTest.kt b/app/k9mail/src/test/java/com/fsck/k9/account/AccountUpdaterTest.kt new file mode 100644 index 0000000000000000000000000000000000000000..8d9ef1ac2ffcba29f0d7326b0f4748db9b1cab38 --- /dev/null +++ b/app/k9mail/src/test/java/com/fsck/k9/account/AccountUpdaterTest.kt @@ -0,0 +1,131 @@ +package com.fsck.k9.account + +import app.k9mail.feature.account.edit.AccountEditExternalContract.AccountUpdaterFailure +import app.k9mail.feature.account.edit.AccountEditExternalContract.AccountUpdaterResult +import assertk.all +import assertk.assertThat +import assertk.assertions.isEqualTo +import assertk.assertions.isInstanceOf +import assertk.assertions.isNotNull +import assertk.assertions.prop +import com.fsck.k9.mail.AuthType +import com.fsck.k9.mail.ConnectionSecurity +import com.fsck.k9.mail.ServerSettings +import kotlinx.coroutines.test.runTest +import org.junit.Test +import com.fsck.k9.Account as K9Account + +class AccountUpdaterTest { + + @Test + fun `updateServerSettings() SHOULD return account not found exception WHEN none present with uuid`() = runTest { + val accountManager = FakeAccountManager(accounts = mutableMapOf()) + val testSubject = AccountServerSettingsUpdater(accountManager) + + val result = testSubject.updateServerSettings( + accountUuid = ACCOUNT_UUID, + isIncoming = true, + serverSettings = INCOMING_SERVER_SETTINGS, + ) + + assertThat(result).isEqualTo(AccountUpdaterResult.Failure(AccountUpdaterFailure.AccountNotFound(ACCOUNT_UUID))) + } + + @Test + fun `updateServerSettings() SHOULD return success with updated incoming settings WHEN is incoming`() = runTest { + val accountManager = FakeAccountManager(accounts = mutableMapOf(ACCOUNT_UUID to createAccount(ACCOUNT_UUID))) + val updatedIncomingServerSettings = INCOMING_SERVER_SETTINGS.copy(port = 123) + val testSubject = AccountServerSettingsUpdater(accountManager) + + val result = testSubject.updateServerSettings( + accountUuid = ACCOUNT_UUID, + isIncoming = true, + serverSettings = updatedIncomingServerSettings, + ) + + assertThat(result).isEqualTo(AccountUpdaterResult.Success(ACCOUNT_UUID)) + + val k9Account = accountManager.getAccount(ACCOUNT_UUID) + assertThat(k9Account).isNotNull().all { + prop(K9Account::incomingServerSettings).isEqualTo(updatedIncomingServerSettings) + prop(K9Account::outgoingServerSettings).isEqualTo(OUTGOING_SERVER_SETTINGS) + } + } + + @Test + fun `updateServerSettings() SHOULD return success with updated outgoing settings WHEN is not incoming`() = runTest { + val accountManager = FakeAccountManager(accounts = mutableMapOf(ACCOUNT_UUID to createAccount(ACCOUNT_UUID))) + val updatedOutgoingServerSettings = OUTGOING_SERVER_SETTINGS.copy(port = 123) + val testSubject = AccountServerSettingsUpdater(accountManager) + + val result = testSubject.updateServerSettings( + accountUuid = ACCOUNT_UUID, + isIncoming = false, + serverSettings = updatedOutgoingServerSettings, + ) + + assertThat(result).isEqualTo(AccountUpdaterResult.Success(ACCOUNT_UUID)) + + val k9Account = accountManager.getAccount(ACCOUNT_UUID) + assertThat(k9Account).isNotNull().all { + prop(K9Account::incomingServerSettings).isEqualTo(INCOMING_SERVER_SETTINGS) + prop(K9Account::outgoingServerSettings).isEqualTo(updatedOutgoingServerSettings) + } + } + + @Test + fun `updateServerSettings() SHOULD return unknown error when exception thrown`() = runTest { + val accountManager = FakeAccountManager( + accounts = mutableMapOf(ACCOUNT_UUID to createAccount(ACCOUNT_UUID)), + isFailureOnSave = true, + ) + val testSubject = AccountServerSettingsUpdater(accountManager) + + val result = testSubject.updateServerSettings( + accountUuid = ACCOUNT_UUID, + isIncoming = true, + serverSettings = INCOMING_SERVER_SETTINGS, + ) + + assertThat(result).isInstanceOf() + .prop(AccountUpdaterResult.Failure::error).isInstanceOf() + .prop(AccountUpdaterFailure.UnknownError::error).isInstanceOf() + } + + private companion object { + const val ACCOUNT_UUID = "uuid" + + val INCOMING_SERVER_SETTINGS = ServerSettings( + type = "imap", + host = "imap.example.org", + port = 143, + connectionSecurity = ConnectionSecurity.SSL_TLS_REQUIRED, + authenticationType = AuthType.PLAIN, + username = "username", + password = "password", + clientCertificateAlias = null, + extra = emptyMap(), + ) + + val OUTGOING_SERVER_SETTINGS = ServerSettings( + type = "smtp", + host = "smtp.example.org", + port = 587, + connectionSecurity = ConnectionSecurity.SSL_TLS_REQUIRED, + authenticationType = AuthType.PLAIN, + username = "username", + password = "password", + clientCertificateAlias = null, + extra = emptyMap(), + ) + + fun createAccount(accountUuid: String): K9Account { + return K9Account( + uuid = accountUuid, + ).apply { + incomingServerSettings = INCOMING_SERVER_SETTINGS + outgoingServerSettings = OUTGOING_SERVER_SETTINGS + } + } + } +} diff --git a/app/k9mail/src/test/java/com/fsck/k9/account/FakeAccountManager.kt b/app/k9mail/src/test/java/com/fsck/k9/account/FakeAccountManager.kt index d22868e2f0b2685560efd7d3f07dd7184f98a9d5..57e78c6f140e37f046ffc82f26ed01c6afa68e36 100644 --- a/app/k9mail/src/test/java/com/fsck/k9/account/FakeAccountManager.kt +++ b/app/k9mail/src/test/java/com/fsck/k9/account/FakeAccountManager.kt @@ -7,7 +7,8 @@ import com.fsck.k9.preferences.AccountManager import kotlinx.coroutines.flow.Flow class FakeAccountManager( - private val accounts: Map = emptyMap(), + private val accounts: MutableMap = mutableMapOf(), + private val isFailureOnSave: Boolean = false, ) : AccountManager { override fun getAccountsFlow(): Flow> { @@ -36,7 +37,11 @@ class FakeAccountManager( TODO("Not yet implemented") } + @Suppress("TooGenericExceptionThrown") override fun saveAccount(account: Account) { - TODO("Not yet implemented") + if (isFailureOnSave) { + throw Exception("FakeAccountManager.saveAccount() failed") + } + accounts[account.uuid] = account } } diff --git a/feature/account/edit/src/main/kotlin/app/k9mail/feature/account/edit/AccountEditExternalContract.kt b/feature/account/edit/src/main/kotlin/app/k9mail/feature/account/edit/AccountEditExternalContract.kt index d7c65cd0e101752b688c9b08f29e3b74e6eebaa9..6ce49b14398aa6634ba7b86089a4d505171c11e9 100644 --- a/feature/account/edit/src/main/kotlin/app/k9mail/feature/account/edit/AccountEditExternalContract.kt +++ b/feature/account/edit/src/main/kotlin/app/k9mail/feature/account/edit/AccountEditExternalContract.kt @@ -1,15 +1,24 @@ package app.k9mail.feature.account.edit -import app.k9mail.feature.account.common.domain.entity.Account +import com.fsck.k9.mail.ServerSettings interface AccountEditExternalContract { - fun interface AccountUpdater { - suspend fun updateAccount(account: Account): AccountUpdaterResult + sealed interface AccountUpdaterResult { + data class Success(val message: String) : AccountUpdaterResult + data class Failure(val error: AccountUpdaterFailure) : AccountUpdaterResult + } + + sealed interface AccountUpdaterFailure { + data class AccountNotFound(val accountUuid: String) : AccountUpdaterFailure + data class UnknownError(val error: Exception) : AccountUpdaterFailure + } - sealed interface AccountUpdaterResult { - data class Success(val accountId: String) : AccountUpdaterResult - data class Error(val message: String) : AccountUpdaterResult - } + fun interface AccountServerSettingsUpdater { + suspend fun updateServerSettings( + accountUuid: String, + isIncoming: Boolean, + serverSettings: ServerSettings, + ): AccountUpdaterResult } }