Skip to content

Commit cba3f91

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 59ffc25 commit cba3f91

14 files changed

Lines changed: 236 additions & 78 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
@@ -62,6 +62,7 @@ internal val mailMessageModule = module {
6262
single<MessageLifecycleRepository> {
6363
DefaultMessageLifecycleRepository(
6464
logger = get(),
65+
messageQueryRepository = get(),
6566
messageStoreManager = get(),
6667
saveMessageDataCreator = get(),
6768
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.logging.Logger
1314
import net.thunderbird.feature.account.AccountId
1415
import net.thunderbird.feature.mail.folder.FolderId
@@ -19,8 +20,10 @@ import net.thunderbird.feature.mail.message.Message
1920
import net.thunderbird.feature.mail.message.MessageDownloadState
2021
import net.thunderbird.feature.mail.message.MessageId
2122
import net.thunderbird.feature.mail.message.MessageServerId
23+
import net.thunderbird.feature.mail.message.domain.GetMessageIdCriteria
2224
import net.thunderbird.feature.mail.message.domain.MessageLifecycleError
2325
import net.thunderbird.feature.mail.message.domain.MessageLifecycleRepository
26+
import net.thunderbird.feature.mail.message.domain.MessageQueryRepository
2427
import net.thunderbird.feature.mail.message.mapper.MessageDataMapper
2528
import com.fsck.k9.mail.Message as LegacyMessageDto
2629
import com.fsck.k9.mail.MessageDownloadState as LegacyMessageDownloadState
@@ -29,6 +32,7 @@ private const val LOG_ID = "[repository][message-lifecycle]"
2932

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

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

73106
override suspend fun move(
@@ -92,21 +125,36 @@ class DefaultMessageLifecycleRepository(
92125
TODO("Not yet implemented")
93126
}
94127

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

109-
private fun createRemoteMessage(
157+
private fun upsertRemoteMessage(
110158
messageStore: ListenableMessageStore,
111159
legacyFolderId: Long,
112160
messageServerId: MessageServerId,

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

Lines changed: 75 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
@@ -92,14 +96,15 @@ class DefaultMessageLifecycleRepositoryTest {
9296
@OptIn(ExperimentalCoroutinesApi::class)
9397
private val testSubject = DefaultMessageLifecycleRepository(
9498
logger = TestLogger(),
99+
messageQueryRepository = messageQueryRepository,
95100
messageStoreManager = messageStoreManager,
96101
saveMessageDataCreator = saveMessageDataCreator,
97102
messageMapper = FakeMessageDataMapper(legacyMessage),
98103
outboxFolderManager = FakeOutboxFolderManager(outboxFolderId = OUTBOX_FOLDER_ID),
99104
ioDispatcher = UnconfinedTestDispatcher(),
100105
)
101106

102-
// region create
107+
// region [create]
103108

104109
@Test
105110
fun `create should save a remote message when it does not exist yet`() = runTest {
@@ -114,6 +119,25 @@ class DefaultMessageLifecycleRepositoryTest {
114119

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

119143
@Test
@@ -164,6 +188,56 @@ class DefaultMessageLifecycleRepositoryTest {
164188
verify(saveMessageDataCreator).createSaveMessageData(legacyMessage, LegacyMessageDownloadState.PARTIAL, null)
165189
}
166190

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

169243
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
@@ -31,7 +28,6 @@ internal class DraftOperations @JvmOverloads constructor(
3128
private val logger: Logger,
3229
private val messagingController: @NotNull MessagingController,
3330
private val messageStoreManager: @NotNull MessageStoreManager,
34-
private val saveMessageDataCreator: SaveMessageDataCreator,
3531
private val localMessageUidPrefixProvider: LocalMessageUidPrefixProvider,
3632
private val messageLifecycleRepository: MessageLifecycleRepository,
3733
private val messageDataMapper: MessageDataMapper<LegacyMessage>,
@@ -68,7 +64,7 @@ internal class DraftOperations @JvmOverloads constructor(
6864
subject: String?,
6965
): Long {
7066
val messageStore = messageStoreManager.getMessageStore(account)
71-
val domainMessage = message.toDomainMessage(subject, account)
67+
val domainMessage = message.toDomainMessage(subject, account, folderId)
7268
val domainFolderId = LegacyFolderIdFactory.of(folderId)
7369
val outcome = messageLifecycleRepository.create(domainMessage, account.id, domainFolderId)
7470
val messageId = outcome.fold(
@@ -103,17 +99,30 @@ internal class DraftOperations @JvmOverloads constructor(
10399
return messageId
104100
}
105101

106-
private fun saveDraftLocally(
102+
private suspend fun saveDraftLocally(
107103
account: LegacyAccountDto,
108104
message: LegacyMessage,
109105
folderId: Long,
110106
existingDraftId: Long?,
111107
plaintextSubject: String?,
112108
): Long {
113-
val messageStore = messageStoreManager.getMessageStore(account)
114-
val messageData = message.toSaveMessageData(plaintextSubject)
115-
116-
return messageStore.saveLocalMessage(folderId, messageData, existingDraftId)
109+
val domainMessage = message.toDomainMessage(plaintextSubject, account, folderId)
110+
val domainFolderId = LegacyFolderIdFactory.of(folderId)
111+
val accountId = account.id
112+
return if (existingDraftId == null) {
113+
messageLifecycleRepository.create(domainMessage, accountId, domainFolderId)
114+
} else {
115+
messageLifecycleRepository.update(
116+
message = domainMessage.copy(id = LegacyMessageIdFactory.of(existingDraftId)),
117+
accountId = accountId,
118+
folderId = domainFolderId,
119+
)
120+
}.fold(
121+
onSuccess = LegacyMessageIdFactory::toLegacyId,
122+
onFailure = { error ->
123+
throw error.throwable ?: MessagingException("Failed to save draft locally: $error")
124+
},
125+
)
117126
}
118127

119128
fun processPendingReplace(command: PendingReplace, account: LegacyAccountDto) {
@@ -186,20 +195,22 @@ internal class DraftOperations @JvmOverloads constructor(
186195
messagingController.destroyPlaceholderMessages(localFolder, messageServerIds)
187196
}
188197

189-
private fun LegacyMessage.toSaveMessageData(subject: String?): SaveMessageData {
190-
return saveMessageDataCreator.createSaveMessageData(this, MessageDownloadState.FULL, subject)
191-
}
192-
193-
private suspend fun LegacyMessage.toDomainMessage(subject: String?, account: LegacyAccountDto): Message {
198+
private suspend fun LegacyMessage.toDomainMessage(
199+
subject: String?,
200+
account: LegacyAccountDto,
201+
folderId: Long,
202+
): Message {
194203
setFlag(Flag.X_DOWNLOADED_FULL, true)
195204
accountId = account.id
196-
val domainMessage = messageDataMapper.toDomain(this).let { domainMessage ->
197-
if (subject.isNullOrBlank()) {
198-
domainMessage
199-
} else {
200-
domainMessage.copy(envelope = domainMessage.envelope.copy(subject = subject))
201-
}
205+
val domainMessage = messageDataMapper.toDomain(this)
206+
val envelop = if (subject.isNullOrBlank()) {
207+
domainMessage.envelope
208+
} else {
209+
domainMessage.envelope.copy(subject = subject)
202210
}
203-
return domainMessage
211+
return domainMessage.copy(
212+
folderId = LegacyFolderIdFactory.of(folderId),
213+
envelope = envelop,
214+
)
204215
}
205216
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ public void run() {
215215
initializeControllerExtensions(controllerExtensions);
216216

217217
draftOperations =
218-
new DraftOperations(Log.INSTANCE, this, messageStoreManager, saveMessageDataCreator, localMessageUidPrefixProvider,
218+
new DraftOperations(Log.INSTANCE, this, messageStoreManager, localMessageUidPrefixProvider,
219219
messageLifecycleRepository, messageDataMapper);
220220
notificationOperations = new NotificationOperations(notificationController, preferences, messageStoreManager);
221221
archiveOperations = new ArchiveOperations(this, featureFlagProvider);

0 commit comments

Comments
 (0)