Skip to content

Commit 007bd78

Browse files
rafaeltonholorafaeltonholo
committed
feat(db-repository): implement update message in MessageLifecycleRepository
- Add update(message, accountId, folderId) to MessageLifecycleRepository with legacy adapter - Add MessageAlreadyExists error and duplicate detection in create() via MessageQueryRepository - Use create/update in K9BackendFolder.saveMessage, recovering from MessageAlreadyExists - Use create/update in DraftOperations.saveDraftLocally - Derive legacy download state from message flags in the mapper - Add update and duplicate-detection tests to DefaultMessageLifecycleRepositoryTest GG-ID: c-a66bc30 GG-Parent: c-639ad93
1 parent 42366db commit 007bd78

14 files changed

Lines changed: 243 additions & 79 deletions

File tree

‎app-common/src/main/kotlin/net/thunderbird/app/common/feature/mail/message/MailMessageModule.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ internal val mailMessageModule = module {
9393
single<MessageLifecycleRepository> {
9494
DefaultMessageLifecycleRepository(
9595
logger = get(),
96+
messageQueryRepository = get(),
9697
messageStoreManager = get(),
9798
saveMessageDataCreator = get(),
9899
messageMapper = get(),

‎app-common/src/main/kotlin/net/thunderbird/app/common/feature/mail/message/data/repository/DefaultMessageLifecycleRepository.kt‎

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import kotlinx.coroutines.Dispatchers
99
import kotlinx.coroutines.withContext
1010
import net.thunderbird.app.common.feature.mail.message.domain.model.LegacyMessageSource
1111
import net.thunderbird.components.core.outcome.Outcome
12+
import net.thunderbird.components.core.outcome.fold
1213
import net.thunderbird.core.architecture.model.LegacyEntityIdFactory
1314
import net.thunderbird.core.logging.Logger
1415
import net.thunderbird.feature.account.AccountId
@@ -18,8 +19,10 @@ import net.thunderbird.feature.mail.message.Message
1819
import net.thunderbird.feature.mail.message.MessageDownloadState
1920
import net.thunderbird.feature.mail.message.MessageId
2021
import net.thunderbird.feature.mail.message.MessageServerId
22+
import net.thunderbird.feature.mail.message.domain.GetMessageIdCriteria
2123
import net.thunderbird.feature.mail.message.domain.MessageLifecycleError
2224
import net.thunderbird.feature.mail.message.domain.MessageLifecycleRepository
25+
import net.thunderbird.feature.mail.message.domain.MessageQueryRepository
2326
import net.thunderbird.feature.mail.message.mapper.MessageDataMapper
2427
import com.fsck.k9.mail.Message as LegacyMessageDto
2528
import com.fsck.k9.mail.MessageDownloadState as LegacyMessageDownloadState
@@ -28,6 +31,7 @@ private const val LOG_ID = "[repository][message-lifecycle]"
2831

2932
class DefaultMessageLifecycleRepository(
3033
private val logger: Logger,
34+
private val messageQueryRepository: MessageQueryRepository,
3135
private val messageStoreManager: MessageStoreManager,
3236
private val saveMessageDataCreator: SaveMessageDataCreator,
3337
private val messageMapper: MessageDataMapper<LegacyMessageDto>,
@@ -41,6 +45,12 @@ class DefaultMessageLifecycleRepository(
4145
accountId: AccountId,
4246
folderId: FolderId?,
4347
): Outcome<MessageId, MessageLifecycleError> = withContext(ioDispatcher) {
48+
val messageServerId = message.serverId
49+
val existingMessageId = verifyMessageExists(folderId, messageServerId, accountId)
50+
if (existingMessageId != null) {
51+
logger.warn { "$LOG_ID message already exists with id '$existingMessageId'. Use update instead." }
52+
return@withContext Outcome.failure(MessageLifecycleError.MessageAlreadyExists(existingMessageId))
53+
}
4454
logger.debug {
4555
"$LOG_ID creating new message in folder '${folderId ?: "<outbox-id>"}' for account '$accountId'"
4656
}
@@ -54,11 +64,10 @@ class DefaultMessageLifecycleRepository(
5464
}
5565
logger.verbose { "$LOG_ID assigning message to folder id = $legacyFolderId" }
5666

57-
val messageServerId = message.serverId
5867
val messageId = if (messageServerId == null) {
59-
createLocalMessage(messageStore, legacyFolderId, messageData)
68+
upsertLocalMessage(messageStore, legacyFolderId, messageData)
6069
} else {
61-
createRemoteMessage(messageStore, legacyFolderId, messageServerId, messageData)
70+
upsertRemoteMessage(messageStore, legacyFolderId, messageServerId, messageData)
6271
}
6372
logger.verbose { "$LOG_ID message created successfully. MessageId = $messageId" }
6473
Outcome.success(messageId)
@@ -67,8 +76,32 @@ class DefaultMessageLifecycleRepository(
6776
override suspend fun update(
6877
message: Message,
6978
accountId: AccountId,
70-
): Outcome<MessageId, MessageLifecycleError> {
71-
TODO("Not yet implemented")
79+
folderId: FolderId,
80+
): Outcome<MessageId, MessageLifecycleError> = withContext(ioDispatcher) {
81+
// NOTE: The legacy store resolves remote messages by folder id, server id (messages.uid), so
82+
// message.id is not strictly needed for that branch today. We still require it so the
83+
// update() contract is the same for local and remote messages and stays valid when
84+
// the data source is replaced.
85+
// Revisit upsertRemoteMessage once the new db can update by message id directly.
86+
val messageId = requireNotNull(message.id) { "$LOG_ID The message.id is required to update a message." }
87+
logger.debug {
88+
"$LOG_ID updating message '$messageId' in folder '$folderId' for account '$accountId'"
89+
}
90+
logger.verbose { "$LOG_ID message = $message" }
91+
val legacyMessage = messageMapper.toDto(message)
92+
val downloadState = message.downloadState.toLegacyDownloadState()
93+
val messageData = saveMessageDataCreator.createSaveMessageData(legacyMessage, downloadState)
94+
val messageStore = messageStoreManager.getMessageStore(accountId.value.toString())
95+
val legacyFolderId = folderIdLegacyEntityIdFactory.toLegacyId(folderId)
96+
val messageServerId = message.serverId
97+
val updatedMessageId = if (messageServerId == null) {
98+
val legacyExistingMessageId = messageIdLegacyEntityIdFactory.toLegacyId(messageId)
99+
upsertLocalMessage(messageStore, legacyFolderId, messageData, legacyExistingMessageId)
100+
} else {
101+
upsertRemoteMessage(messageStore, legacyFolderId, messageServerId, messageData)
102+
}
103+
logger.verbose { "$LOG_ID message updated successfully. MessageId = $updatedMessageId" }
104+
Outcome.success(updatedMessageId)
72105
}
73106

74107
override suspend fun move(
@@ -93,21 +126,36 @@ class DefaultMessageLifecycleRepository(
93126
TODO("Not yet implemented")
94127
}
95128

96-
private fun createLocalMessage(
129+
private suspend fun verifyMessageExists(
130+
folderId: FolderId?,
131+
messageServerId: MessageServerId?,
132+
accountId: AccountId,
133+
): MessageId? {
134+
return if (folderId != null && messageServerId != null) {
135+
messageQueryRepository
136+
.findIdByCriteria(accountId, GetMessageIdCriteria(folderId, messageServerId))
137+
.fold(onSuccess = { it }, onFailure = { throw it.throwable })
138+
} else {
139+
null
140+
}
141+
}
142+
143+
private fun upsertLocalMessage(
97144
messageStore: ListenableMessageStore,
98145
legacyFolderId: Long,
99146
messageData: SaveMessageData,
147+
existingLegacyMessageId: Long? = null,
100148
): MessageId {
101149
logger.verbose { "$LOG_ID creating local message in folder id '$legacyFolderId'" }
102150
val legacyMessageId = messageStore.saveLocalMessage(
103151
folderId = legacyFolderId,
104152
messageData = messageData,
105-
existingMessageId = null,
153+
existingMessageId = existingLegacyMessageId,
106154
)
107155
return messageIdLegacyEntityIdFactory.of(legacyMessageId)
108156
}
109157

110-
private fun createRemoteMessage(
158+
private fun upsertRemoteMessage(
111159
messageStore: ListenableMessageStore,
112160
legacyFolderId: Long,
113161
messageServerId: MessageServerId,

‎app-common/src/test/kotlin/net/thunderbird/app/common/feature/mail/message/data/repository/DefaultMessageLifecycleRepositoryTest.kt‎

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@ import app.k9mail.legacy.mailstore.MessageStoreFactory
66
import app.k9mail.legacy.mailstore.MessageStoreManager
77
import app.k9mail.legacy.mailstore.SaveMessageData
88
import app.k9mail.legacy.message.extractors.PreviewResult
9+
import assertk.assertFailure
910
import assertk.assertThat
1011
import assertk.assertions.isEqualTo
12+
import assertk.assertions.isInstanceOf
1113
import com.fsck.k9.mail.internet.MimeMessage
1214
import com.fsck.k9.mailstore.SaveMessageDataCreator
1315
import kotlin.test.Test
@@ -39,11 +41,13 @@ import net.thunderbird.feature.mail.message.MessageHeaders
3941
import net.thunderbird.feature.mail.message.MessageId
4042
import net.thunderbird.feature.mail.message.MessageServerId
4143
import net.thunderbird.feature.mail.message.domain.GetMessageIdCriteria
44+
import net.thunderbird.feature.mail.message.domain.MessageLifecycleError
4245
import net.thunderbird.feature.mail.message.domain.MessageQueryError
4346
import net.thunderbird.feature.mail.message.domain.MessageQueryRepository
4447
import net.thunderbird.feature.mail.message.mapper.MessageDataMapper
4548
import org.mockito.kotlin.mock
4649
import org.mockito.kotlin.verify
50+
import org.mockito.kotlin.verifyNoInteractions
4751
import org.mockito.kotlin.whenever
4852
import com.fsck.k9.mail.Message as LegacyMessageDto
4953
import com.fsck.k9.mail.MessageDownloadState as LegacyMessageDownloadState
@@ -53,6 +57,7 @@ private const val OUTBOX_FOLDER_ID = 3L
5357
private const val EXISTING_MESSAGE_ID = 42L
5458
private const val SAVED_MESSAGE_ID = 5L
5559
private const val SERVER_ID = "100"
60+
private const val UNKNOWN_ACCOUNT_ID = "22222222-2222-2222-2222-222222222222"
5661

5762
/**
5863
* [MessageStore] and [SaveMessageDataCreator] are mocked here: the store is a 60+ method legacy interface and the
@@ -91,6 +96,7 @@ class DefaultMessageLifecycleRepositoryTest {
9196
@OptIn(ExperimentalCoroutinesApi::class)
9297
private val testSubject = DefaultMessageLifecycleRepository(
9398
logger = TestLogger(),
99+
messageQueryRepository = messageQueryRepository,
94100
messageStoreManager = messageStoreManager,
95101
saveMessageDataCreator = saveMessageDataCreator,
96102
messageMapper = FakeMessageDataMapper(legacyMessage),
@@ -100,7 +106,7 @@ class DefaultMessageLifecycleRepositoryTest {
100106
ioDispatcher = UnconfinedTestDispatcher(),
101107
)
102108

103-
// region create
109+
// region [create]
104110

105111
@Test
106112
fun `create should save a remote message when it does not exist yet`() = runTest {
@@ -115,6 +121,25 @@ class DefaultMessageLifecycleRepositoryTest {
115121

116122
// Assert
117123
assertThat(outcome).isEqualTo(Outcome.success(LegacyMessageIdFactory.of(SAVED_MESSAGE_ID)))
124+
assertThat(messageQueryRepository.lastCriteria)
125+
.isEqualTo(GetMessageIdCriteria(folderId, MessageServerId(SERVER_ID)))
126+
verify(messageStore).saveRemoteMessage(FOLDER_ID, SERVER_ID, saveMessageData)
127+
}
128+
129+
@Test
130+
fun `create should return MessageAlreadyExists without touching the store when the message exists`() = runTest {
131+
// Arrange
132+
val existingMessageId = LegacyMessageIdFactory.of(EXISTING_MESSAGE_ID)
133+
messageQueryRepository.result = Outcome.success(existingMessageId)
134+
val message = buildDomainMessage(serverId = MessageServerId(SERVER_ID))
135+
136+
// Act
137+
val outcome = testSubject.create(message, accountId, folderId)
138+
139+
// Assert
140+
assertThat(outcome).isEqualTo(Outcome.failure(MessageLifecycleError.MessageAlreadyExists(existingMessageId)))
141+
verifyNoInteractions(messageStore)
142+
verifyNoInteractions(saveMessageDataCreator)
118143
}
119144

120145
@Test
@@ -165,6 +190,56 @@ class DefaultMessageLifecycleRepositoryTest {
165190
verify(saveMessageDataCreator).createSaveMessageData(legacyMessage, LegacyMessageDownloadState.PARTIAL, null)
166191
}
167192

193+
// endregion [create]
194+
195+
// region [update]
196+
197+
@Test
198+
fun `update should replace the existing local message when the message has no server id`() = runTest {
199+
// Arrange
200+
val message = buildDomainMessage(id = LegacyMessageIdFactory.of(EXISTING_MESSAGE_ID), serverId = null)
201+
stubSaveMessageData()
202+
whenever(messageStore.saveLocalMessage(FOLDER_ID, saveMessageData, EXISTING_MESSAGE_ID))
203+
.thenReturn(EXISTING_MESSAGE_ID)
204+
205+
// Act
206+
val outcome = testSubject.update(message, accountId, folderId)
207+
208+
// Assert
209+
assertThat(outcome).isEqualTo(Outcome.success(LegacyMessageIdFactory.of(EXISTING_MESSAGE_ID)))
210+
verify(messageStore).saveLocalMessage(FOLDER_ID, saveMessageData, EXISTING_MESSAGE_ID)
211+
}
212+
213+
@Test
214+
fun `update should save the remote message by folder and server id`() = runTest {
215+
// Arrange
216+
val message = buildDomainMessage(
217+
id = LegacyMessageIdFactory.of(EXISTING_MESSAGE_ID),
218+
serverId = MessageServerId(SERVER_ID),
219+
)
220+
stubSaveMessageData()
221+
whenever(messageStore.saveRemoteMessage(FOLDER_ID, SERVER_ID, saveMessageData))
222+
.thenReturn(EXISTING_MESSAGE_ID)
223+
224+
// Act
225+
val outcome = testSubject.update(message, accountId, folderId)
226+
227+
// Assert
228+
assertThat(outcome).isEqualTo(Outcome.success(LegacyMessageIdFactory.of(EXISTING_MESSAGE_ID)))
229+
verify(messageStore).saveRemoteMessage(FOLDER_ID, SERVER_ID, saveMessageData)
230+
}
231+
232+
@Test
233+
fun `update should fail fast when the message has no id`() = runTest {
234+
// Arrange
235+
val message = buildDomainMessage(id = null, serverId = null)
236+
237+
// Act & Assert
238+
assertFailure { testSubject.update(message, accountId, folderId) }
239+
.isInstanceOf<IllegalArgumentException>()
240+
verifyNoInteractions(messageStore)
241+
}
242+
168243
// endregion
169244

170245
private fun stubSaveMessageData() {

‎feature/mail/message/api/src/commonMain/kotlin/net/thunderbird/feature/mail/message/domain/MessageLifecycleRepository.kt‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,12 @@ interface MessageLifecycleRepository {
2525
folderId: FolderId? = null,
2626
): Outcome<MessageId, MessageLifecycleError>
2727

28-
suspend fun update(message: Message, accountId: AccountId): Outcome<MessageId, MessageLifecycleError>
28+
suspend fun update(
29+
message: Message,
30+
accountId: AccountId,
31+
folderId: FolderId,
32+
): Outcome<MessageId, MessageLifecycleError>
33+
2934
suspend fun move(messageId: MessageId, destinationFolderId: FolderId): Outcome<MessageId, MessageLifecycleError>
3035
suspend fun copy(messageId: MessageId, destinationFolderId: FolderId): Outcome<MessageId, MessageLifecycleError>
3136
suspend fun destroy(
@@ -37,4 +42,10 @@ interface MessageLifecycleRepository {
3742

3843
sealed interface MessageLifecycleError {
3944
val throwable: Throwable?
45+
46+
data class MessageAlreadyExists(val messageId: MessageId) : MessageLifecycleError {
47+
override val throwable: Throwable? = null
48+
}
49+
50+
data class UnhandledError(override val throwable: Throwable) : MessageLifecycleError
4051
}

‎legacy/core/src/main/java/com/fsck/k9/controller/DraftOperations.kt‎

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
11
package com.fsck.k9.controller
22

33
import app.k9mail.legacy.mailstore.MessageStoreManager
4-
import app.k9mail.legacy.mailstore.SaveMessageData
54
import com.fsck.k9.backend.api.Backend
65
import com.fsck.k9.controller.MessagingControllerCommands.PendingAppend
76
import com.fsck.k9.controller.MessagingControllerCommands.PendingReplace
87
import com.fsck.k9.mail.FetchProfile
9-
import com.fsck.k9.mail.MessageDownloadState
108
import com.fsck.k9.mailstore.LocalFolder
119
import com.fsck.k9.mailstore.LocalMessage
12-
import com.fsck.k9.mailstore.SaveMessageDataCreator
1310
import kotlinx.coroutines.CoroutineDispatcher
1411
import kotlinx.coroutines.Dispatchers
1512
import kotlinx.coroutines.runBlocking
@@ -32,7 +29,6 @@ internal class DraftOperations @JvmOverloads constructor(
3229
private val logger: Logger,
3330
private val messagingController: @NotNull MessagingController,
3431
private val messageStoreManager: @NotNull MessageStoreManager,
35-
private val saveMessageDataCreator: SaveMessageDataCreator,
3632
private val localMessageUidPrefixProvider: LocalMessageUidPrefixProvider,
3733
private val messageLifecycleRepository: MessageLifecycleRepository,
3834
private val messageDataMapper: MessageDataMapper<LegacyMessage>,
@@ -71,7 +67,7 @@ internal class DraftOperations @JvmOverloads constructor(
7167
subject: String?,
7268
): Long {
7369
val messageStore = messageStoreManager.getMessageStore(account)
74-
val domainMessage = message.toDomainMessage(subject, account)
70+
val domainMessage = message.toDomainMessage(subject, account, folderId)
7571
val domainFolderId = folderIdFactory.of(folderId)
7672
val outcome = messageLifecycleRepository.create(domainMessage, account.id, domainFolderId)
7773
val messageId = outcome.fold(
@@ -106,17 +102,30 @@ internal class DraftOperations @JvmOverloads constructor(
106102
return messageId
107103
}
108104

109-
private fun saveDraftLocally(
105+
private suspend fun saveDraftLocally(
110106
account: LegacyAccountDto,
111107
message: LegacyMessage,
112108
folderId: Long,
113109
existingDraftId: Long?,
114110
plaintextSubject: String?,
115111
): Long {
116-
val messageStore = messageStoreManager.getMessageStore(account)
117-
val messageData = message.toSaveMessageData(plaintextSubject)
118-
119-
return messageStore.saveLocalMessage(folderId, messageData, existingDraftId)
112+
val domainMessage = message.toDomainMessage(plaintextSubject, account, folderId)
113+
val domainFolderId = folderIdFactory.of(folderId)
114+
val accountId = account.id
115+
return if (existingDraftId == null) {
116+
messageLifecycleRepository.create(domainMessage, accountId, domainFolderId)
117+
} else {
118+
messageLifecycleRepository.update(
119+
message = domainMessage.copy(id = messageIdFactory.of(existingDraftId)),
120+
accountId = accountId,
121+
folderId = domainFolderId,
122+
)
123+
}.fold(
124+
onSuccess = messageIdFactory::toLegacyId,
125+
onFailure = { error ->
126+
throw error.throwable ?: MessagingException("Failed to save draft locally: $error")
127+
},
128+
)
120129
}
121130

122131
fun processPendingReplace(command: PendingReplace, account: LegacyAccountDto) {
@@ -189,20 +198,22 @@ internal class DraftOperations @JvmOverloads constructor(
189198
messagingController.destroyPlaceholderMessages(localFolder, messageServerIds)
190199
}
191200

192-
private fun LegacyMessage.toSaveMessageData(subject: String?): SaveMessageData {
193-
return saveMessageDataCreator.createSaveMessageData(this, MessageDownloadState.FULL, subject)
194-
}
195-
196-
private suspend fun LegacyMessage.toDomainMessage(subject: String?, account: LegacyAccountDto): Message {
201+
private suspend fun LegacyMessage.toDomainMessage(
202+
subject: String?,
203+
account: LegacyAccountDto,
204+
folderId: Long,
205+
): Message {
197206
setFlag(Flag.X_DOWNLOADED_FULL, true)
198207
setAccountUuid(account.uuid)
199-
val domainMessage = messageDataMapper.toDomain(this).let { domainMessage ->
200-
if (subject.isNullOrBlank()) {
201-
domainMessage
202-
} else {
203-
domainMessage.copy(envelope = domainMessage.envelope.copy(subject = subject))
204-
}
208+
val domainMessage = messageDataMapper.toDomain(this)
209+
val envelop = if (subject.isNullOrBlank()) {
210+
domainMessage.envelope
211+
} else {
212+
domainMessage.envelope.copy(subject = subject)
205213
}
206-
return domainMessage
214+
return domainMessage.copy(
215+
folderId = folderIdFactory.of(folderId),
216+
envelope = envelop,
217+
)
207218
}
208219
}

0 commit comments

Comments
 (0)