Skip to content

Commit 2d4df33

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 413a52d commit 2d4df33

14 files changed

Lines changed: 244 additions & 80 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
@@ -81,6 +81,7 @@ internal val mailMessageModule = module {
8181
single<MessageLifecycleRepository> {
8282
DefaultMessageLifecycleRepository(
8383
logger = get(),
84+
messageQueryRepository = get(),
8485
messageStoreManager = get(),
8586
saveMessageDataCreator = get(),
8687
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)
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
@@ -40,11 +42,13 @@ import net.thunderbird.feature.mail.message.MessageHeaders
4042
import net.thunderbird.feature.mail.message.MessageId
4143
import net.thunderbird.feature.mail.message.MessageServerId
4244
import net.thunderbird.feature.mail.message.domain.GetMessageIdCriteria
45+
import net.thunderbird.feature.mail.message.domain.MessageLifecycleError
4346
import net.thunderbird.feature.mail.message.domain.MessageQueryError
4447
import net.thunderbird.feature.mail.message.domain.MessageQueryRepository
4548
import net.thunderbird.feature.mail.message.mapper.MessageDataMapper
4649
import org.mockito.kotlin.mock
4750
import org.mockito.kotlin.verify
51+
import org.mockito.kotlin.verifyNoInteractions
4852
import org.mockito.kotlin.whenever
4953
import com.fsck.k9.mail.Message as LegacyMessageDto
5054
import com.fsck.k9.mail.MessageDownloadState as LegacyMessageDownloadState
@@ -54,6 +58,7 @@ private const val OUTBOX_FOLDER_ID = 3L
5458
private const val EXISTING_MESSAGE_ID = 42L
5559
private const val SAVED_MESSAGE_ID = 5L
5660
private const val SERVER_ID = "100"
61+
private const val UNKNOWN_ACCOUNT_ID = "22222222-2222-2222-2222-222222222222"
5762

5863
/**
5964
* [MessageStore] and [SaveMessageDataCreator] are mocked here: the store is a 60+ method legacy interface and the
@@ -92,6 +97,7 @@ class DefaultMessageLifecycleRepositoryTest {
9297
@OptIn(ExperimentalCoroutinesApi::class)
9398
private val testSubject = DefaultMessageLifecycleRepository(
9499
logger = TestLogger(),
100+
messageQueryRepository = messageQueryRepository,
95101
messageStoreManager = messageStoreManager,
96102
saveMessageDataCreator = saveMessageDataCreator,
97103
messageMapper = FakeMessageDataMapper(legacyMessage),
@@ -101,7 +107,7 @@ class DefaultMessageLifecycleRepositoryTest {
101107
ioDispatcher = UnconfinedTestDispatcher(),
102108
)
103109

104-
// region create
110+
// region [create]
105111

106112
@Test
107113
fun `create should save a remote message when it does not exist yet`() = runTest {
@@ -116,6 +122,25 @@ class DefaultMessageLifecycleRepositoryTest {
116122

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

121146
@Test
@@ -166,6 +191,56 @@ class DefaultMessageLifecycleRepositoryTest {
166191
verify(saveMessageDataCreator).createSaveMessageData(legacyMessage, LegacyMessageDownloadState.PARTIAL, null)
167192
}
168193

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

171246
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: 34 additions & 23 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)
198-
setAccountId(account.id)
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-
}
207+
accountId = account.id
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)