wbrawner/thunderbird-android
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Properly report progress when receiving multiple FETCH responses per message

Browse source
This commit is contained in:
cketti 2022-03-05 01:13:46 +01:00
parent 9dd076686b
commit 556188efcc
7 changed files with 99 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

22
backend/imap/src/main/java/com/fsck/k9/backend/imap/ImapSync.kt
View file

@ -13,8 +13,8 @@ import com.fsck.k9.mail.DefaultBodyFactory
import com.fsck.k9.mail.FetchProfile
import com.fsck.k9.mail.Flag
import com.fsck.k9.mail.MessageDownloadState
import com.fsck.k9.mail.MessageRetrievalListener
import com.fsck.k9.mail.internet.MessageExtractor
import com.fsck.k9.mail.store.imap.FetchListener
import com.fsck.k9.mail.store.imap.ImapFolder
import com.fsck.k9.mail.store.imap.ImapMessage
import com.fsck.k9.mail.store.imap.ImapStore
@ -452,15 +452,18 @@ internal class ImapSync(
remoteFolder.fetch(
unsyncedMessages,
fetchProfile,
object : MessageRetrievalListener<ImapMessage> {
override fun messageFinished(message: ImapMessage) {
object : FetchListener {
override fun onFetchResponse(message: ImapMessage, isFirstResponse: Boolean) {
try {
if (message.isSet(Flag.DELETED)) {
Timber.v(
"Newly downloaded message %s:%s:%s was marked deleted on server, skipping",
accountName, folder, message.uid
)
progress.incrementAndGet()
if (isFirstResponse) {
progress.incrementAndGet()
}
// TODO: This might be the source of poll count errors in the UI. Is todo always the same as ofTotal
listener.syncProgress(folder, progress.get(), todo)
@ -504,13 +507,16 @@ internal class ImapSync(
remoteFolder.fetch(
smallMessages,
fetchProfile,
object : MessageRetrievalListener<ImapMessage> {
override fun messageFinished(message: ImapMessage) {
object : FetchListener {
override fun onFetchResponse(message: ImapMessage, isFirstResponse: Boolean) {
try {
// Store the updated message locally
backendFolder.saveMessage(message, MessageDownloadState.FULL)
progress.incrementAndGet()
downloadedMessageCount.incrementAndGet()
if (isFirstResponse) {
progress.incrementAndGet()
downloadedMessageCount.incrementAndGet()
}
val messageServerId = message.uid
Timber.v(

67
backend/imap/src/test/java/com/fsck/k9/backend/imap/ImapSyncTest.kt
View file

@ -5,11 +5,14 @@ import com.fsck.k9.backend.api.FolderInfo
import com.fsck.k9.backend.api.SyncConfig
import com.fsck.k9.backend.api.SyncConfig.ExpungePolicy
import com.fsck.k9.backend.api.SyncListener
import com.fsck.k9.mail.FetchProfile
import com.fsck.k9.mail.Flag
import com.fsck.k9.mail.FolderType
import com.fsck.k9.mail.Message
import com.fsck.k9.mail.MessageDownloadState
import com.fsck.k9.mail.buildMessage
import com.fsck.k9.mail.store.imap.FetchListener
import com.fsck.k9.mail.store.imap.ImapMessage
import com.google.common.truth.Truth.assertThat
import java.util.Date
import org.apache.james.mime4j.dom.field.DateTimeField
@ -17,6 +20,7 @@ import org.apache.james.mime4j.field.DefaultFieldParser
import org.junit.Test
import org.mockito.Mockito.verify
import org.mockito.kotlin.any
import org.mockito.kotlin.atLeast
import org.mockito.kotlin.eq
import org.mockito.kotlin.mock
import org.mockito.kotlin.never
@ -208,6 +212,35 @@ class ImapSyncTest {
verify(syncListener).syncNewMessage(FOLDER_SERVER_ID, messageServerId = "1", isOldMessage = false)
}
@Test
fun `sync with multiple FETCH responses when downloading small message should report correct progress`() {
val folderServerId = "FOLDER_TWO"
backendStorage.createBackendFolder(folderServerId)
val specialImapFolder = object : TestImapFolder(folderServerId) {
override fun fetch(
messages: List<ImapMessage>,
fetchProfile: FetchProfile,
listener: FetchListener?,
maxDownloadSize: Int
) {
super.fetch(messages, fetchProfile, listener, maxDownloadSize)
// When fetching the body simulate an additional FETCH response
if (FetchProfile.Item.BODY in fetchProfile) {
val message = messages.first()
listener?.onFetchResponse(message, isFirstResponse = false)
}
}
}
specialImapFolder.addMessage(42)
imapStore.addFolder(specialImapFolder)
imapSync.sync(folderServerId, defaultSyncConfig, syncListener)
verify(syncListener, atLeast(1)).syncProgress(folderServerId, completed = 1, total = 1)
verify(syncListener, never()).syncProgress(folderServerId, completed = 2, total = 1)
}
private fun addMessageToBackendFolder(uid: Long, date: String = DEFAULT_MESSAGE_DATE) {
val messageServerId = uid.toString()
val message = createSimpleMessage(messageServerId, date).apply {
@ -222,13 +255,21 @@ class ImapSyncTest {
}
private fun addMessageToImapFolder(uid: Long, flags: Set<Flag> = emptySet(), date: String = DEFAULT_MESSAGE_DATE) {
imapFolder.addMessage(uid, flags, date)
}
private fun TestImapFolder.addMessage(
uid: Long,
flags: Set<Flag> = emptySet(),
date: String = DEFAULT_MESSAGE_DATE
) {
val messageServerId = uid.toString()
val message = createSimpleMessage(messageServerId, date)
imapFolder.addMessage(uid, message)
addMessage(uid, message)
if (flags.isNotEmpty()) {
val imapMessage = imapFolder.getMessage(messageServerId)
imapFolder.setFlags(listOf(imapMessage), flags, true)
val imapMessage = getMessage(messageServerId)
setFlags(listOf(imapMessage), flags, true)
}
}
@ -239,14 +280,18 @@ class ImapSyncTest {
private fun createBackendStorage(): InMemoryBackendStorage {
return InMemoryBackendStorage().apply {
createFolderUpdater().use { updater ->
val folderInfo = FolderInfo(
serverId = FOLDER_SERVER_ID,
name = "irrelevant",
type = FolderType.REGULAR
)
updater.createFolders(listOf(folderInfo))
}
createBackendFolder(FOLDER_SERVER_ID)
}
}
private fun InMemoryBackendStorage.createBackendFolder(serverId: String) {
createFolderUpdater().use { updater ->
val folderInfo = FolderInfo(
serverId = serverId,
name = "irrelevant",
type = FolderType.REGULAR
)
updater.createFolders(listOf(folderInfo))
}
}

9
backend/imap/src/test/java/com/fsck/k9/backend/imap/TestImapFolder.kt
View file

@ -6,15 +6,16 @@ import com.fsck.k9.mail.Flag
import com.fsck.k9.mail.Message
import com.fsck.k9.mail.MessageRetrievalListener
import com.fsck.k9.mail.Part
import com.fsck.k9.mail.store.imap.FetchListener
import com.fsck.k9.mail.store.imap.ImapFolder
import com.fsck.k9.mail.store.imap.ImapMessage
import com.fsck.k9.mail.store.imap.OpenMode
import com.fsck.k9.mail.store.imap.createImapMessage
import java.util.Date
class TestImapFolder(override val serverId: String) : ImapFolder {
open class TestImapFolder(override val serverId: String) : ImapFolder {
override var mode: OpenMode? = null
private set
protected set
override var messageCount: Int = 0
@ -92,7 +93,7 @@ class TestImapFolder(override val serverId: String) : ImapFolder {
override fun fetch(
messages: List<ImapMessage>,
fetchProfile: FetchProfile,
listener: MessageRetrievalListener<ImapMessage>?,
listener: FetchListener?,
maxDownloadSize: Int
) {
if (messages.isEmpty()) return
@ -109,7 +110,7 @@ class TestImapFolder(override val serverId: String) : ImapFolder {
}
imapMessage.body = storedMessage.body
listener?.messageFinished(imapMessage)
listener?.onFetchResponse(imapMessage, isFirstResponse = true)
}
}

17
backend/imap/src/test/java/com/fsck/k9/backend/imap/TestImapStore.kt
View file

@ -6,16 +6,23 @@ import com.fsck.k9.mail.store.imap.ImapFolder
import com.fsck.k9.mail.store.imap.ImapStore
class TestImapStore : ImapStore {
private val folders = mutableMapOf<String, TestImapFolder>()
private val folders = mutableMapOf<String, ImapFolder>()
fun addFolder(name: String): TestImapFolder {
require(!folders.containsKey(name)) { "Folder '$name' already exists" }
fun addFolder(serverId: String): TestImapFolder {
require(!folders.containsKey(serverId)) { "Folder '$serverId' already exists" }
return TestImapFolder(name).also { folder ->
folders[name] = folder
return TestImapFolder(serverId).also { folder ->
folders[serverId] = folder
}
}
fun addFolder(folder: ImapFolder) {
val serverId = folder.serverId
require(!folders.containsKey(serverId)) { "Folder '$serverId' already exists" }
folders[serverId] = folder
}
override fun getFolder(name: String): ImapFolder {
return folders[name] ?: error("Folder '$name' not found")
}

6
mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/ImapFolder.kt
View file

@ -45,7 +45,7 @@ interface ImapFolder {
fun fetch(
messages: List<ImapMessage>,
fetchProfile: FetchProfile,
listener: MessageRetrievalListener<ImapMessage>?,
listener: FetchListener?,
maxDownloadSize: Int
)
@ -86,3 +86,7 @@ interface ImapFolder {
@Throws(MessagingException::class)
fun expungeUids(uids: List<String>)
}
interface FetchListener {
fun onFetchResponse(message: ImapMessage, isFirstResponse: Boolean)
}

8
mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/RealImapFolder.kt
View file

@ -515,7 +515,7 @@ internal class RealImapFolder(
override fun fetch(
messages: List<ImapMessage>,
fetchProfile: FetchProfile,
listener: MessageRetrievalListener<ImapMessage>?,
listener: FetchListener?,
maxDownloadSize: Int
) {
if (messages.isEmpty()) {
@ -561,6 +561,7 @@ internal class RealImapFolder(
val spaceSeparatedFetchFields = ImapUtility.join(" ", fetchFields)
var windowStart = 0
val processedUids = mutableSetOf<String>()
while (windowStart < messages.size) {
val windowEnd = min(windowStart + FETCH_WINDOW_SIZE, messages.size)
val uidWindow = uids.subList(windowStart, windowEnd)
@ -610,7 +611,10 @@ internal class RealImapFolder(
}
}
listener?.messageFinished(message)
val isFirstResponse = uid !in processedUids
processedUids.add(uid)
listener?.onFetchResponse(message, isFirstResponse)
} else {
handleUntaggedResponse(response)
}

2
mail/protocols/imap/src/test/java/com/fsck/k9/mail/store/imap/TestImapFolder.kt
View file

@ -68,7 +68,7 @@ internal open class TestImapFolder(
override fun fetch(
messages: List<ImapMessage>,
fetchProfile: FetchProfile,
listener: MessageRetrievalListener<ImapMessage>?,
listener: FetchListener?,
maxDownloadSize: Int
) {
throw UnsupportedOperationException("not implemented")