Skip to content

Commit 4de312d

Browse files
rafaeltonholorafaeltonholo
committed
feat(db-repository): implement move message in MessageLifecycleRepository
- Add move(messageId, destinationFolderId, accountId) and moveAll() to MessageLifecycleRepository - Implement both in DefaultMessageLifecycleRepository with legacy adapter, mapping store exceptions to UnhandledError - Replace MessageStore.moveMessage/moveMessages calls in MessagingController with repository move/moveAll - Add move/moveAll tests to DefaultMessageLifecycleRepositoryTest and sent-message move tests to MessagingControllerTest GG-ID: c-7f7af18 GG-Parent: c-a66bc30
1 parent 1501a6a commit 4de312d

6 files changed

Lines changed: 376 additions & 29 deletions

File tree

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

Lines changed: 103 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import app.k9mail.legacy.mailstore.ListenableMessageStore
44
import app.k9mail.legacy.mailstore.MessageStoreManager
55
import app.k9mail.legacy.mailstore.SaveMessageData
66
import com.fsck.k9.mailstore.SaveMessageDataCreator
7+
import kotlin.coroutines.cancellation.CancellationException
78
import kotlinx.coroutines.CoroutineDispatcher
89
import kotlinx.coroutines.Dispatchers
910
import kotlinx.coroutines.withContext
@@ -55,7 +56,11 @@ class DefaultMessageLifecycleRepository(
5556
"$LOG_ID creating new message in folder '${folderId ?: "<outbox-id>"}' for account '$accountId'"
5657
}
5758
logger.verbose { "$LOG_ID message = $message" }
58-
val messageData = message.toSaveMessageData()
59+
val messageData = message.toSaveMessageData(
60+
logger = logger,
61+
saveMessageDataCreator = saveMessageDataCreator,
62+
messageDataMapper = messageMapper,
63+
)
5964
val messageStore = messageStoreManager.getMessageStore(accountId)
6065
val legacyFolderId = if (folderId == null) {
6166
outboxFolderManager.getOutboxFolderId(accountId)
@@ -107,8 +112,61 @@ class DefaultMessageLifecycleRepository(
107112
override suspend fun move(
108113
messageId: MessageId,
109114
destinationFolderId: FolderId,
110-
): Outcome<MessageId, MessageLifecycleError> {
111-
TODO("Not yet implemented")
115+
accountId: AccountId,
116+
): Outcome<MessageId, MessageLifecycleError> = withContext(ioDispatcher) {
117+
logger.debug { "$LOG_ID moving '$messageId' to folder '$destinationFolderId' for account '$accountId'" }
118+
val legacyMessageId = messageIdLegacyEntityIdFactory.toLegacyId(messageId)
119+
val legacyFolderId = folderIdLegacyEntityIdFactory.toLegacyId(destinationFolderId)
120+
logger.verbose {
121+
"$LOG_ID message id '$messageId' -> legacy id '$legacyMessageId', " +
122+
"folder id '$destinationFolderId' -> legacy id '$legacyFolderId'"
123+
}
124+
125+
runLegacy(operation = "move message '$messageId'") {
126+
val messageStore = messageStoreManager.getMessageStore(accountId)
127+
val legacyDestinationMessageId = messageStore.moveMessage(legacyMessageId, legacyFolderId)
128+
messageIdLegacyEntityIdFactory.of(legacyDestinationMessageId).also { destinationMessageId ->
129+
logger.verbose { "$LOG_ID moved message '$messageId' -> '$destinationMessageId'" }
130+
}
131+
}
132+
}
133+
134+
override suspend fun moveAll(
135+
messageIds: List<MessageId>,
136+
destinationFolderId: FolderId,
137+
accountId: AccountId,
138+
): Outcome<Map<MessageId, MessageId>, MessageLifecycleError> = withContext(ioDispatcher) {
139+
if (messageIds.isEmpty()) {
140+
logger.verbose { "$LOG_ID nothing to move to folder '$destinationFolderId' for account '$accountId'" }
141+
return@withContext Outcome.success(emptyMap())
142+
}
143+
logger.debug {
144+
"$LOG_ID moving ${messageIds.size} messages to folder '$destinationFolderId' for account '$accountId'"
145+
}
146+
val legacyMessageIds = messageIds.map(messageIdLegacyEntityIdFactory::toLegacyId)
147+
val legacyFolderId = folderIdLegacyEntityIdFactory.toLegacyId(destinationFolderId)
148+
logger.verbose {
149+
"$LOG_ID message ids $messageIds -> legacy ids $legacyMessageIds, " +
150+
"folder id '$destinationFolderId' -> legacy id '$legacyFolderId'"
151+
}
152+
153+
runLegacy(operation = "move ${messageIds.size} messages") {
154+
val messageStore = messageStoreManager.getMessageStore(accountId)
155+
messageStore.moveMessages(legacyMessageIds, legacyFolderId)
156+
.entries
157+
.associate { (sourceLegacyId, destinationLegacyId) ->
158+
messageIdLegacyEntityIdFactory.of(sourceLegacyId) to
159+
messageIdLegacyEntityIdFactory.of(destinationLegacyId)
160+
}
161+
.also { mapping ->
162+
logger.verbose {
163+
val lines = mapping.entries.joinToString("\n") { (source, destination) ->
164+
"'$source' -> '$destination'"
165+
}
166+
"$LOG_ID moved messages to folder '$destinationFolderId':\n${lines.prependIndent(" ")}"
167+
}
168+
}
169+
}
112170
}
113171

114172
override suspend fun copy(
@@ -126,6 +184,23 @@ class DefaultMessageLifecycleRepository(
126184
TODO("Not yet implemented")
127185
}
128186

187+
/**
188+
* Runs a legacy store operation and converts any thrown exception into a [MessageLifecycleError.UnhandledError].
189+
* [CancellationException] is rethrown so structured concurrency keeps working.
190+
*/
191+
@Suppress("TooGenericExceptionCaught")
192+
private inline fun <T> runLegacy(
193+
operation: String,
194+
block: () -> T,
195+
): Outcome<T, MessageLifecycleError> = try {
196+
Outcome.success(block())
197+
} catch (e: CancellationException) {
198+
throw e
199+
} catch (e: Exception) {
200+
logger.error(throwable = e) { "$LOG_ID $operation failed" }
201+
Outcome.failure(MessageLifecycleError.UnhandledError(e))
202+
}
203+
129204
private suspend fun verifyMessageExists(
130205
folderId: FolderId?,
131206
messageServerId: MessageServerId?,
@@ -169,31 +244,35 @@ class DefaultMessageLifecycleRepository(
169244
)
170245
return messageIdLegacyEntityIdFactory.of(legacyMessageId)
171246
}
247+
}
172248

173-
private fun MessageDownloadState.toLegacyDownloadState(): LegacyMessageDownloadState = when (this) {
174-
MessageDownloadState.ENVELOPE -> LegacyMessageDownloadState.ENVELOPE
175-
MessageDownloadState.PARTIAL -> LegacyMessageDownloadState.PARTIAL
176-
MessageDownloadState.FULL -> LegacyMessageDownloadState.FULL
177-
}
249+
private fun MessageDownloadState.toLegacyDownloadState(): LegacyMessageDownloadState = when (this) {
250+
MessageDownloadState.ENVELOPE -> LegacyMessageDownloadState.ENVELOPE
251+
MessageDownloadState.PARTIAL -> LegacyMessageDownloadState.PARTIAL
252+
MessageDownloadState.FULL -> LegacyMessageDownloadState.FULL
253+
}
178254

179-
private suspend fun Message.toSaveMessageData(): SaveMessageData {
180-
val downloadState = downloadState.toLegacyDownloadState()
181-
return when (val source = source) {
182-
is LegacyMessageSource -> {
183-
logger.verbose {
184-
"$LOG_ID convert message -> save message data using message source ($source)"
185-
}
186-
saveMessageDataCreator.createSaveMessageData(
187-
source.message,
188-
downloadState,
189-
envelope.subject,
190-
)
255+
private suspend fun Message.toSaveMessageData(
256+
logger: Logger,
257+
saveMessageDataCreator: SaveMessageDataCreator,
258+
messageDataMapper: MessageDataMapper<LegacyMessageDto>,
259+
): SaveMessageData {
260+
val downloadState = downloadState.toLegacyDownloadState()
261+
return when (val source = source) {
262+
is LegacyMessageSource -> {
263+
logger.verbose {
264+
"$LOG_ID convert message -> save message data using message source ($source)"
191265
}
266+
saveMessageDataCreator.createSaveMessageData(
267+
source.message,
268+
downloadState,
269+
envelope.subject,
270+
)
271+
}
192272

193-
else -> {
194-
logger.verbose { "$LOG_ID convert message -> save message data using message mapper" }
195-
saveMessageDataCreator.createSaveMessageData(messageMapper.toDto(this), downloadState)
196-
}
273+
else -> {
274+
logger.verbose { "$LOG_ID convert message -> save message data using message mapper" }
275+
saveMessageDataCreator.createSaveMessageData(messageDataMapper.toDto(this), downloadState)
197276
}
198277
}
199278
}

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

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import net.thunderbird.core.android.account.LegacyAccountDto
2929
import net.thunderbird.core.android.account.LegacyAccountDtoManager
3030
import net.thunderbird.core.logging.testing.TestLogger
3131
import net.thunderbird.feature.account.AccountId
32+
import net.thunderbird.feature.account.AccountIdFactory
3233
import net.thunderbird.feature.mail.folder.FolderId
3334
import net.thunderbird.feature.mail.folder.LegacyFolderIdFactory
3435
import net.thunderbird.feature.mail.folder.api.OutboxFolderManager
@@ -243,6 +244,126 @@ class DefaultMessageLifecycleRepositoryTest {
243244

244245
// endregion
245246

247+
// region [move]
248+
249+
@Test
250+
fun `move should return the destination message id when the store succeeds`() = runTest {
251+
// Arrange
252+
whenever(messageStore.moveMessage(EXISTING_MESSAGE_ID, FOLDER_ID)).thenReturn(SAVED_MESSAGE_ID)
253+
254+
// Act
255+
val outcome = testSubject.move(LegacyMessageIdFactory.of(EXISTING_MESSAGE_ID), folderId, accountId)
256+
257+
// Assert
258+
assertThat(outcome).isEqualTo(Outcome.success(LegacyMessageIdFactory.of(SAVED_MESSAGE_ID)))
259+
verify(messageStore).moveMessage(EXISTING_MESSAGE_ID, FOLDER_ID)
260+
}
261+
262+
@Test
263+
fun `move should return UnhandledError when the store throws`() = runTest {
264+
// Arrange
265+
val exception = IllegalStateException("Couldn't find local message [ID: $EXISTING_MESSAGE_ID]")
266+
whenever(messageStore.moveMessage(EXISTING_MESSAGE_ID, FOLDER_ID)).thenThrow(exception)
267+
268+
// Act
269+
val outcome = testSubject.move(LegacyMessageIdFactory.of(EXISTING_MESSAGE_ID), folderId, accountId)
270+
271+
// Assert
272+
assertThat(outcome).isEqualTo(Outcome.failure(MessageLifecycleError.UnhandledError(exception)))
273+
}
274+
275+
@Test
276+
fun `move should return UnhandledError when the account is unknown`() = runTest {
277+
// Act
278+
val outcome = testSubject.move(
279+
messageId = LegacyMessageIdFactory.of(EXISTING_MESSAGE_ID),
280+
destinationFolderId = folderId,
281+
accountId = AccountIdFactory.of(UNKNOWN_ACCOUNT_ID),
282+
)
283+
284+
// Assert
285+
assertThat(outcome).isInstanceOf<Outcome.Failure<MessageLifecycleError>>()
286+
.transform { it.error }
287+
.isInstanceOf<MessageLifecycleError.UnhandledError>()
288+
.transform { it.throwable }
289+
.isInstanceOf<IllegalStateException>()
290+
verifyNoInteractions(messageStore)
291+
}
292+
293+
// endregion [move]
294+
295+
// region [moveAll]
296+
297+
@Test
298+
fun `moveAll should return an empty mapping without touching the store when given no ids`() = runTest {
299+
// Act
300+
val outcome = testSubject.moveAll(emptyList(), folderId, accountId)
301+
302+
// Assert
303+
assertThat(outcome).isEqualTo(Outcome.success(emptyMap<MessageId, MessageId>()))
304+
verifyNoInteractions(messageStore)
305+
}
306+
307+
@Test
308+
fun `moveAll should map source ids to destination ids when the store succeeds`() = runTest {
309+
// Arrange
310+
whenever(messageStore.moveMessages(listOf(1L, 2L), FOLDER_ID)).thenReturn(mapOf(1L to 11L, 2L to 12L))
311+
312+
// Act
313+
val outcome = testSubject.moveAll(
314+
messageIds = listOf(LegacyMessageIdFactory.of(1L), LegacyMessageIdFactory.of(2L)),
315+
destinationFolderId = folderId,
316+
accountId = accountId,
317+
)
318+
319+
// Assert
320+
assertThat(outcome).isEqualTo(
321+
Outcome.success(
322+
mapOf(
323+
LegacyMessageIdFactory.of(1L) to LegacyMessageIdFactory.of(11L),
324+
LegacyMessageIdFactory.of(2L) to LegacyMessageIdFactory.of(12L),
325+
),
326+
),
327+
)
328+
}
329+
330+
@Test
331+
fun `moveAll should return UnhandledError when the store throws`() = runTest {
332+
// Arrange
333+
val exception = IllegalStateException("Couldn't find local message [ID: 2]")
334+
whenever(messageStore.moveMessages(listOf(1L, 2L), FOLDER_ID)).thenThrow(exception)
335+
336+
// Act
337+
val outcome = testSubject.moveAll(
338+
messageIds = listOf(LegacyMessageIdFactory.of(1L), LegacyMessageIdFactory.of(2L)),
339+
destinationFolderId = folderId,
340+
accountId = accountId,
341+
)
342+
343+
// Assert
344+
assertThat(outcome).isEqualTo(Outcome.failure(MessageLifecycleError.UnhandledError(exception)))
345+
}
346+
347+
@Test
348+
fun `moveAll should return UnhandledError when the account is unknown`() = runTest {
349+
// Act
350+
val outcome = testSubject.moveAll(
351+
messageIds = listOf(LegacyMessageIdFactory.of(1L)),
352+
destinationFolderId = folderId,
353+
accountId = AccountIdFactory.of(UNKNOWN_ACCOUNT_ID),
354+
)
355+
356+
// Assert
357+
assertThat(outcome).isInstanceOf<Outcome.Failure<MessageLifecycleError>>()
358+
.transform { it.error }
359+
.isInstanceOf<MessageLifecycleError.UnhandledError>()
360+
.transform { it.throwable }
361+
.isInstanceOf<IllegalStateException>()
362+
verifyNoInteractions(messageStore)
363+
}
364+
365+
// endregion [moveAll]
366+
246367
private fun stubSaveMessageData() {
247368
whenever(saveMessageDataCreator.createSaveMessageData(legacyMessage, LegacyMessageDownloadState.FULL, null))
248369
.thenReturn(saveMessageData)

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

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,42 @@ interface MessageLifecycleRepository {
3131
folderId: FolderId,
3232
): Outcome<MessageId, MessageLifecycleError>
3333

34-
suspend fun move(messageId: MessageId, destinationFolderId: FolderId): Outcome<MessageId, MessageLifecycleError>
34+
/**
35+
* Moves a locally stored message to another folder.
36+
*
37+
* The message is stored under a new id in the destination folder. The source entry is kept as a placeholder
38+
* so threading information survives until the remote side confirms the move.
39+
*
40+
* @param messageId The id of the message to move.
41+
* @param destinationFolderId The folder the message is moved to.
42+
* @param accountId The account both the message and the destination folder belong to.
43+
* @return An [Outcome] containing the [MessageId] of the message in the destination folder on success, or a
44+
* [MessageLifecycleError] on failure. The returned id differs from [messageId].
45+
*/
46+
suspend fun move(
47+
messageId: MessageId,
48+
destinationFolderId: FolderId,
49+
accountId: AccountId,
50+
): Outcome<MessageId, MessageLifecycleError>
51+
52+
/**
53+
* Moves several locally stored messages to another folder. See [move] for the semantics of a single move.
54+
*
55+
* This operation is not atomic: messages are moved one by one. If a move fails, messages processed before the
56+
* failure stay in the destination folder and the returned [Outcome] is a failure without any mapping.
57+
*
58+
* @param messageIds The ids of the messages to move. An empty list results in an empty mapping.
59+
* @param destinationFolderId The folder the messages are moved to.
60+
* @param accountId The account the messages and the destination folder belong to.
61+
* @return An [Outcome] containing a mapping from each source [MessageId] to the [MessageId] of the message in
62+
* the destination folder on success, or a [MessageLifecycleError] on failure.
63+
*/
64+
suspend fun moveAll(
65+
messageIds: List<MessageId>,
66+
destinationFolderId: FolderId,
67+
accountId: AccountId,
68+
): Outcome<Map<MessageId, MessageId>, MessageLifecycleError>
69+
3570
suspend fun copy(messageId: MessageId, destinationFolderId: FolderId): Outcome<MessageId, MessageLifecycleError>
3671
suspend fun destroy(
3772
serverIds: List<MessageServerId>,

0 commit comments

Comments
 (0)