Skip to content

Commit 2025f1e

Browse files
wmontwewmontwe
committed
fix(storage): read accounts from the cached storage snapshot
1 parent a0cc5c5 commit 2025f1e

13 files changed

Lines changed: 205 additions & 71 deletions

File tree

‎app-common/build.gradle.kts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ dependencies {
7474
testImplementation(projects.core.testing)
7575
testImplementation(projects.core.android.testing)
7676
testImplementation(projects.core.configstore.testing)
77+
testImplementation(projects.legacy.storage)
7778

7879
testImplementation(libs.mockito.kotlin)
7980
}

‎app-common/src/main/kotlin/net/thunderbird/app/common/account/data/DefaultAccountDisplayOrderManager.kt‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,14 @@
11
package net.thunderbird.app.common.account.data
22

3-
import net.thunderbird.core.preference.storage.Storage
43
import net.thunderbird.core.preference.storage.StorageProvider
54
import net.thunderbird.feature.account.AccountId
65

76
class DefaultAccountDisplayOrderManager(
87
private val storageProvider: StorageProvider,
98
) : AccountDisplayOrderManager {
109

11-
private val storage: Storage
12-
get() = storageProvider.loadLatestStorage()
13-
1410
override fun moveToPosition(accountId: AccountId, newPosition: Int) {
15-
val accountUuids = storage.getStringOrDefault("accountUuids", "")
11+
val accountUuids = storageProvider.storage.getStringOrDefault("accountUuids", "")
1612
.split(",")
1713
.filter { it.isNotEmpty() }
1814

‎app-common/src/main/kotlin/net/thunderbird/app/common/account/data/LegacyAccountLocalDataSource.kt‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package net.thunderbird.app.common.account.data
22

33
import net.thunderbird.core.android.account.AccountDefaultsProvider
44
import net.thunderbird.core.android.account.LegacyAccount
5-
import net.thunderbird.core.preference.storage.Storage
65
import net.thunderbird.core.preference.storage.StorageProvider
76
import net.thunderbird.feature.account.AccountId
87
import net.thunderbird.feature.account.AccountIdFactory
@@ -14,11 +13,8 @@ internal class LegacyAccountLocalDataSource(
1413
private val accountDefaultsProvider: AccountDefaultsProvider,
1514
) : AccountLocalDataSource {
1615

17-
// Reads the whole preferences table, so each operation reads it once and passes it on
18-
private fun loadStorage(): Storage = storageProvider.loadLatestStorage()
19-
2016
override fun loadAll(): List<LegacyAccount> {
21-
val storage = loadStorage()
17+
val storage = storageProvider.storage
2218
val accountUuids = storage.getStringOrNull("accountUuids") ?: return emptyList()
2319

2420
return accountUuids.split(",")
@@ -31,7 +27,7 @@ internal class LegacyAccountLocalDataSource(
3127
}
3228

3329
override fun getById(accountId: AccountId): LegacyAccount? {
34-
val storage = loadStorage()
30+
val storage = storageProvider.storage
3531
val accountUuids = storage.getStringOrNull("accountUuids")?.split(",")
3632
if (accountUuids == null || accountId.toString() !in accountUuids) return null
3733

@@ -41,13 +37,13 @@ internal class LegacyAccountLocalDataSource(
4137

4238
override fun save(account: LegacyAccount) {
4339
val editor = storageProvider.createStorageEditor()
44-
accountStorageHandler.save(account, loadStorage(), editor)
40+
accountStorageHandler.save(account, storageProvider.storage, editor)
4541
check(editor.commit()) { "Failed to save account settings" }
4642
}
4743

4844
override fun delete(accountId: AccountId) {
4945
val editor = storageProvider.createStorageEditor()
50-
accountStorageHandler.delete(accountId, loadStorage(), editor)
46+
accountStorageHandler.delete(accountId, storageProvider.storage, editor)
5147
check(editor.commit()) { "Failed to delete account settings" }
5248
}
5349
}

‎app-common/src/test/kotlin/net/thunderbird/app/common/account/data/LegacyAccountLocalDataSourceStorageTest.kt‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
package net.thunderbird.app.common.account.data
2+
3+
import assertk.assertThat
4+
import assertk.assertions.containsExactly
5+
import assertk.assertions.isEqualTo
6+
import assertk.assertions.isNotNull
7+
import assertk.assertions.isNull
8+
import assertk.assertions.prop
9+
import com.fsck.k9.mail.AuthType
10+
import com.fsck.k9.mail.ConnectionSecurity
11+
import com.fsck.k9.mail.ServerSettings
12+
import com.fsck.k9.preferences.K9StoragePersister
13+
import net.thunderbird.components.core.logging.testing.TestLogger
14+
import net.thunderbird.core.android.account.AccountDefaultsProvider
15+
import net.thunderbird.core.android.account.LegacyAccount
16+
import net.thunderbird.core.android.testing.RobolectricTest
17+
import net.thunderbird.core.preference.storage.DefaultStorageProvider
18+
import net.thunderbird.core.preference.storage.Storage
19+
import net.thunderbird.core.preference.storage.StorageProvider
20+
import net.thunderbird.feature.account.AccountId
21+
import net.thunderbird.feature.account.AccountIdFactory
22+
import net.thunderbird.feature.account.storage.legacy.LegacyAccountStorageHandler
23+
import net.thunderbird.feature.account.storage.legacy.LegacyAvatarDtoStorageHandler
24+
import net.thunderbird.feature.account.storage.legacy.LegacyProfileDtoStorageHandler
25+
import net.thunderbird.feature.account.storage.legacy.serializer.ServerSettingsDtoSerializer
26+
import net.thunderbird.feature.account.storage.profile.AvatarDto
27+
import net.thunderbird.feature.account.storage.profile.AvatarTypeDto
28+
import net.thunderbird.feature.account.storage.profile.ProfileDto
29+
import org.junit.Test
30+
import org.robolectric.RuntimeEnvironment
31+
32+
/**
33+
* Runs [LegacyAccountLocalDataSource] against the real preferences database to verify that reading from the cached
34+
* storage snapshot returns the latest values and that nothing is lost on disk.
35+
*/
36+
class LegacyAccountLocalDataSourceStorageTest : RobolectricTest() {
37+
38+
private val logger = TestLogger()
39+
40+
private val storageProvider = createStorageProvider()
41+
private val testSubject = createTestSubject(storageProvider)
42+
43+
@Test
44+
fun `getById should return the latest saved values`() {
45+
val account = createAccount()
46+
47+
testSubject.save(account.copy(lastSyncTime = 1L))
48+
testSubject.save(account.copy(lastSyncTime = 2L))
49+
50+
assertThat(testSubject.getById(account.id)).isNotNull()
51+
.prop(LegacyAccount::lastSyncTime).isEqualTo(2L)
52+
}
53+
54+
@Test
55+
fun `getById should return account values written by another editor`() {
56+
val account = createAccount()
57+
testSubject.save(account)
58+
59+
storageProvider.createStorageEditor()
60+
.putLong("${account.id}.lastSyncTime", 42L)
61+
.commit()
62+
63+
assertThat(testSubject.getById(account.id)).isNotNull()
64+
.prop(LegacyAccount::lastSyncTime).isEqualTo(42L)
65+
}
66+
67+
@Test
68+
fun `saved values should be loaded after a restart`() {
69+
val account = createAccount()
70+
testSubject.save(account.copy(lastSyncTime = 1L))
71+
testSubject.save(account.copy(lastSyncTime = 2L))
72+
73+
val restartedTestSubject = createTestSubject(createStorageProvider())
74+
75+
assertThat(restartedTestSubject.loadAll().map { it.id }).containsExactly(account.id)
76+
assertThat(restartedTestSubject.getById(account.id)).isNotNull()
77+
.prop(LegacyAccount::lastSyncTime).isEqualTo(2L)
78+
}
79+
80+
@Test
81+
fun `saving an account should keep values written by another editor`() {
82+
val account = createAccount()
83+
testSubject.save(account)
84+
85+
storageProvider.createStorageEditor()
86+
.putString("otherKey", "otherValue")
87+
.commit()
88+
testSubject.save(account.copy(lastSyncTime = 1L))
89+
90+
val restartedStorageProvider = createStorageProvider()
91+
assertThat(restartedStorageProvider.storage.getStringOrNull("otherKey")).isEqualTo("otherValue")
92+
assertThat(createTestSubject(restartedStorageProvider).getById(account.id)).isNotNull()
93+
.prop(LegacyAccount::lastSyncTime).isEqualTo(1L)
94+
}
95+
96+
@Test
97+
fun `deleted account should be gone after a restart`() {
98+
val account = createAccount()
99+
val otherAccount = createAccount()
100+
testSubject.save(account)
101+
testSubject.save(otherAccount)
102+
103+
testSubject.delete(account.id)
104+
105+
val restartedTestSubject = createTestSubject(createStorageProvider())
106+
assertThat(restartedTestSubject.getById(account.id)).isNull()
107+
assertThat(restartedTestSubject.loadAll().map { it.id }).containsExactly(otherAccount.id)
108+
}
109+
110+
private fun createStorageProvider(): StorageProvider {
111+
return DefaultStorageProvider(K9StoragePersister(RuntimeEnvironment.getApplication(), logger))
112+
}
113+
114+
private fun createTestSubject(storageProvider: StorageProvider): LegacyAccountLocalDataSource {
115+
return LegacyAccountLocalDataSource(
116+
storageProvider = storageProvider,
117+
accountStorageHandler = LegacyAccountStorageHandler(
118+
serverSettingsDtoSerializer = ServerSettingsDtoSerializer(),
119+
profileDtoStorageHandler = LegacyProfileDtoStorageHandler(LegacyAvatarDtoStorageHandler()),
120+
logger = logger,
121+
),
122+
accountDefaultsProvider = PassThroughAccountDefaultsProvider(),
123+
)
124+
}
125+
126+
private class PassThroughAccountDefaultsProvider : AccountDefaultsProvider {
127+
override fun applyDefaults(account: LegacyAccount): LegacyAccount = account
128+
override fun applyOverwrites(account: LegacyAccount, storage: Storage): LegacyAccount = account
129+
}
130+
131+
private companion object {
132+
fun createAccount(id: AccountId = AccountIdFactory.create()): LegacyAccount {
133+
return LegacyAccount(
134+
id = id,
135+
name = "Account",
136+
email = "user@example.com",
137+
profile = ProfileDto(
138+
id = id,
139+
name = "Account",
140+
color = -1,
141+
avatar = AvatarDto(
142+
accountId = id,
143+
avatarType = AvatarTypeDto.MONOGRAM,
144+
avatarMonogram = "AC",
145+
avatarImageUri = null,
146+
avatarIconName = null,
147+
),
148+
),
149+
incomingServerSettings = createServerSettings("imap", 993),
150+
outgoingServerSettings = createServerSettings("smtp", 465),
151+
identities = emptyList(),
152+
accountNumber = 1,
153+
)
154+
}
155+
156+
fun createServerSettings(type: String, port: Int) = ServerSettings(
157+
type = type,
158+
host = "$type.example.com",
159+
port = port,
160+
connectionSecurity = ConnectionSecurity.SSL_TLS_REQUIRED,
161+
authenticationType = AuthType.PLAIN,
162+
username = "user",
163+
password = "password",
164+
clientCertificateAlias = null,
165+
)
166+
}
167+
}

‎app-common/src/test/kotlin/net/thunderbird/app/common/account/data/LegacyAccountLocalDataSourceTest.kt‎

Lines changed: 1 addition & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -54,34 +54,6 @@ class LegacyAccountLocalDataSourceTest {
5454
assertThat(result).containsExactly(account2, account1, account3)
5555
}
5656

57-
@Test
58-
fun `loadAll should read the storage once for all accounts`() {
59-
// Arrange
60-
val accountIds = List(3) { AccountIdFactory.create() }
61-
accountIds.forEach { fakeAccountStorageHandler.setAccount(createFakeAccount(it, "Account $it")) }
62-
val testSubject = createTestSubject(FakeStorage(mapOf("accountUuids" to accountIds.joinToString(","))))
63-
64-
// Act
65-
testSubject.loadAll()
66-
67-
// Assert
68-
assertThat(fakeStorageProvider.loadLatestStorageCalls).isEqualTo(1)
69-
}
70-
71-
@Test
72-
fun `getById should read the storage once`() {
73-
// Arrange
74-
val accountId = AccountIdFactory.create()
75-
fakeAccountStorageHandler.setAccount(createFakeAccount(accountId, "Account"))
76-
val testSubject = createTestSubject(FakeStorage(mapOf("accountUuids" to accountId.toString())))
77-
78-
// Act
79-
testSubject.getById(accountId)
80-
81-
// Assert
82-
assertThat(fakeStorageProvider.loadLatestStorageCalls).isEqualTo(1)
83-
}
84-
8557
@Test
8658
fun `loadAll should return empty list when accountUuids is missing`() {
8759
val testSubject = createTestSubject(FakeStorage(emptyMap()))
@@ -265,14 +237,8 @@ class LegacyAccountLocalDataSourceTest {
265237
val committedValues = mutableMapOf<String, String?>()
266238
val committedRemovals = mutableListOf<String>()
267239

268-
var loadLatestStorageCalls = 0
269-
private set
270-
271240
override val storage: Storage get() = storageInstance
272-
override fun loadLatestStorage(): Storage {
273-
loadLatestStorageCalls++
274-
return storageInstance
275-
}
241+
override fun loadLatestStorage(): Storage = storageInstance
276242
override fun createStorageEditor(): StorageEditor = FakeStorageEditor(this)
277243
}
278244

‎app-k9mail/src/test/kotlin/app/k9mail/DependencyInjectionTest.kt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import net.openid.appauth.AppAuthConfiguration
2626
import net.thunderbird.app.common.feature.account.AccountRemoverWorker
2727
import net.thunderbird.core.common.mail.html.HtmlSettings
2828
import net.thunderbird.core.configstore.ConfigId
29-
import net.thunderbird.core.preference.storage.Storage
3029
import net.thunderbird.feature.account.AccountId
3130
import net.thunderbird.feature.changelog.internal.ChangelogViewModel
3231
import net.thunderbird.feature.mail.message.list.ui.dialog.SetupArchiveFolderDialogContract
@@ -57,7 +56,6 @@ class DependencyInjectionTest {
5756
InteractionMode::class,
5857
NotificationManager::class,
5958
Resources::class,
60-
Storage::class,
6159
ConfigId::class,
6260
),
6361
injections = injectedParameters(

‎app-thunderbird/src/test/kotlin/net/thunderbird/android/DependencyInjectionTest.kt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import net.openid.appauth.AppAuthConfiguration
2626
import net.thunderbird.app.common.feature.account.AccountRemoverWorker
2727
import net.thunderbird.core.common.mail.html.HtmlSettings
2828
import net.thunderbird.core.configstore.ConfigId
29-
import net.thunderbird.core.preference.storage.Storage
3029
import net.thunderbird.feature.account.AccountId
3130
import net.thunderbird.feature.changelog.internal.ChangelogViewModel
3231
import net.thunderbird.feature.mail.message.list.ui.dialog.SetupArchiveFolderDialogContract
@@ -57,7 +56,6 @@ class DependencyInjectionTest {
5756
InteractionMode::class,
5857
NotificationManager::class,
5958
Resources::class,
60-
Storage::class,
6159
ConfigId::class,
6260
),
6361
injections = injectedParameters(

‎core/preference/impl/src/commonTest/kotlin/net/thunderbird/core/preference/storage/DefaultStorageProviderTest.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,5 +51,6 @@ class DefaultStorageProviderTest {
5151
editor.commit()
5252

5353
assertThat(provider.storage.getStringOrDefault("key1", "")).isEqualTo("value1")
54+
assertThat(provider.storage.getStringOrDefault("key2", "")).isEqualTo("value2")
5455
}
5556
}

‎feature/changelog/internal/src/main/kotlin/net/thunderbird/feature/changelog/internal/ChangelogVersionHistory.kt‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,7 @@ import kotlinx.coroutines.sync.Mutex
1111
import kotlinx.coroutines.sync.withLock
1212
import net.thunderbird.components.core.logging.Logger
1313
import net.thunderbird.core.preference.storage.Storage
14-
import net.thunderbird.core.preference.storage.StorageEditor
15-
import net.thunderbird.core.preference.storage.StoragePersister
14+
import net.thunderbird.core.preference.storage.StorageProvider
1615

1716
private const val TAG = "ChangelogTracker"
1817
private const val FIRST_VERSION_KEY: String = "ckChangeLog_first_version_code"
@@ -23,16 +22,15 @@ class ChangelogVersionHistory(
2322
private val context: Context,
2423
private val changeLogProvider: ChangelogProvider,
2524
private val logger: Logger,
26-
private val storagePersister: StoragePersister,
27-
private val storageEditor: StorageEditor,
25+
private val storageProvider: StorageProvider,
2826
private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO,
2927
private var scope: CoroutineScope = CoroutineScope(SupervisorJob()),
3028
) {
3129
val changeLog by lazy { changeLogProvider.getChangeLog() }
3230
val recentChanges by lazy { changeLogProvider.getChangeLogSince(lastVersionCode) }
3331
private val mutex = Mutex()
3432
private val storage: Storage
35-
get() = storagePersister.loadValues()
33+
get() = storageProvider.loadLatestStorage()
3634

3735
private var firstVersionCode = 0
3836
private var lastVersionCode = 0
@@ -56,6 +54,7 @@ class ChangelogVersionHistory(
5654
firstVersionCode = if (lastVersionCode != NO_VERSION) lastVersionCode else currentVersionCode
5755
scope.launch(ioDispatcher) {
5856
mutex.withLock {
57+
val storageEditor = storageProvider.createStorageEditor()
5958
storageEditor.putInt(FIRST_VERSION_KEY, firstVersionCode)
6059
storageEditor.putInt(LAST_VERSION_KEY, firstVersionCode)
6160
storageEditor.commit()
@@ -84,6 +83,7 @@ class ChangelogVersionHistory(
8483

8584
scope.launch(ioDispatcher) {
8685
mutex.withLock {
86+
val storageEditor = storageProvider.createStorageEditor()
8787
storageEditor.putInt(LAST_VERSION_KEY, currentVersionCode)
8888
storageEditor.commit()
8989
logger.debug(TAG) { "saved: currentVersionCode: $currentVersionCode" }

‎feature/changelog/internal/src/main/kotlin/net/thunderbird/feature/changelog/internal/KoinModule.kt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@ val changelogUiModule = module {
1313
context = get(),
1414
changeLogProvider = get(),
1515
logger = get(),
16-
storagePersister = get(),
17-
storageEditor = get(),
16+
storageProvider = get(),
1817
)
1918
}
2019
single { ChangeLogManager(changelogVersionHistory = get(), appCoroutineScope = get(named("AppCoroutineScope"))) }

0 commit comments

Comments
 (0)