From 5693c898f6da5b48050e12604dcaf7f2cc9ae8ae Mon Sep 17 00:00:00 2001 From: cketti Date: Fri, 14 Feb 2020 18:18:11 +0100 Subject: [PATCH] Add JMAP message flags/keywords sync --- .../com/fsck/k9/backend/jmap/CommandSync.kt | 77 ++++++++++++++----- .../com/fsck/k9/backend/jmap/JmapBackend.kt | 2 +- .../fsck/k9/backend/jmap/CommandSyncTest.kt | 46 ++++++++--- .../k9/backend/jmap/InMemoryBackendFolder.kt | 14 +++- .../k9/backend/jmap/LoggingSyncListener.kt | 12 +++ .../email_get_keywords_M001_and_M002.json | 26 +++++++ .../email/email_get_keywords_M002.json | 20 +++++ 7 files changed, 161 insertions(+), 36 deletions(-) create mode 100644 backend/jmap/src/test/resources/jmap_responses/email/email_get_keywords_M001_and_M002.json create mode 100644 backend/jmap/src/test/resources/jmap_responses/email/email_get_keywords_M002.json diff --git a/backend/jmap/src/main/java/com/fsck/k9/backend/jmap/CommandSync.kt b/backend/jmap/src/main/java/com/fsck/k9/backend/jmap/CommandSync.kt index ef90aef3a..3fef1ab22 100644 --- a/backend/jmap/src/main/java/com/fsck/k9/backend/jmap/CommandSync.kt +++ b/backend/jmap/src/main/java/com/fsck/k9/backend/jmap/CommandSync.kt @@ -2,6 +2,7 @@ package com.fsck.k9.backend.jmap import com.fsck.k9.backend.api.BackendFolder import com.fsck.k9.backend.api.BackendStorage +import com.fsck.k9.backend.api.SyncConfig import com.fsck.k9.backend.api.SyncListener import com.fsck.k9.mail.AuthenticationFailedException import com.fsck.k9.mail.Flag @@ -35,7 +36,7 @@ class CommandSync( private val httpAuthentication: HttpAuthentication ) { - fun sync(folderServerId: String, listener: SyncListener) { + fun sync(folderServerId: String, syncConfig: SyncConfig, listener: SyncListener) { try { val backendFolder = backendStorage.getFolder(folderServerId) listener.syncStarted(folderServerId) @@ -44,9 +45,9 @@ class CommandSync( val queryState = backendFolder.getFolderExtraString(EXTRA_QUERY_STATE) if (queryState == null) { - fullSync(backendFolder, folderServerId, limit, listener) + fullSync(backendFolder, folderServerId, syncConfig, limit, listener) } else { - deltaSync(backendFolder, folderServerId, limit, queryState, listener) + deltaSync(backendFolder, folderServerId, syncConfig, limit, queryState, listener) } listener.syncFinished(folderServerId) @@ -62,7 +63,13 @@ class CommandSync( } } - private fun fullSync(backendFolder: BackendFolder, folderServerId: String, limit: Long?, listener: SyncListener) { + private fun fullSync( + backendFolder: BackendFolder, + folderServerId: String, + syncConfig: SyncConfig, + limit: Long?, + listener: SyncListener + ) { val cachedServerIds: Set = backendFolder.getMessageServerIds() if (limit != null) { @@ -82,7 +89,8 @@ class CommandSync( handleFolderUpdates(backendFolder, folderServerId, destroyServerIds, newServerIds, queryState, listener) - // TODO: Refresh flags of messages we've already downloaded before + val refreshServerIds = cachedServerIds.intersect(remoteServerIds) + refreshMessageFlags(backendFolder, syncConfig, refreshServerIds) } private fun createEmailQuery(folderServerId: String): EmailQuery? { @@ -97,6 +105,7 @@ class CommandSync( private fun deltaSync( backendFolder: BackendFolder, folderServerId: String, + syncConfig: SyncConfig, limit: Long?, queryState: String, listener: SyncListener @@ -114,20 +123,23 @@ class CommandSync( Timber.d("Server responded with '$ERROR_CANNOT_CALCULATE_CHANGES'; switching to full sync") backendFolder.saveQueryState(null) - fullSync(backendFolder, folderServerId, limit, listener) + fullSync(backendFolder, folderServerId, syncConfig, limit, listener) return } throw e } + val cachedServerIds = backendFolder.getMessageServerIds() + val destroyServerIds = queryChangesEmailResponse.removed.toList() val newServerIds = queryChangesEmailResponse.added.map { it.item }.toSet() val newQueryState = queryChangesEmailResponse.newQueryState handleFolderUpdates(backendFolder, folderServerId, destroyServerIds, newServerIds, newQueryState, listener) - // TODO: Refresh flags of messages we've already downloaded before + val refreshServerIds = cachedServerIds - destroyServerIds + refreshMessageFlags(backendFolder, syncConfig, refreshServerIds) } private fun handleFolderUpdates( @@ -179,7 +191,7 @@ class CommandSync( private fun fetchMessageInfo(session: Session, maxObjectsInGet: Int, emailIds: Set): List { return emailIds .chunked(maxObjectsInGet) { emailIdsChunk -> - fetchEmailsForMessageInfo(emailIdsChunk) + getEmailPropertiesFromServer(emailIdsChunk, INFO_PROPERTIES) } .flatten() .map { email -> @@ -187,19 +199,8 @@ class CommandSync( } } - private fun fetchEmailsForMessageInfo(emailIdsChunk: List): List { - val getEmailMethod = GetEmailMethodCall( - accountId, - emailIdsChunk.toTypedArray(), - arrayOf( - "id", - "blobId", - "size", - "receivedAt", - "keywords" - ) - ) - + private fun getEmailPropertiesFromServer(emailIdsChunk: List, properties: Array): List { + val getEmailMethod = GetEmailMethodCall(accountId, emailIdsChunk.toTypedArray(), properties) val getEmailCall = jmapClient.call(getEmailMethod) val getEmailResponse = getEmailCall.getMainResponseBlocking() @@ -229,6 +230,38 @@ class CommandSync( } } + private fun refreshMessageFlags(backendFolder: BackendFolder, syncConfig: SyncConfig, emailIds: Set) { + if (emailIds.isEmpty()) return + + Timber.v("Fetching flags for messages: %s", emailIds) + + val session = jmapClient.session.get() + val maxObjectsInGet = session.maxObjectsInGet() + + emailIds + .asSequence() + .chunked(maxObjectsInGet) { emailIdsChunk -> + getEmailPropertiesFromServer(emailIdsChunk, FLAG_PROPERTIES) + } + .flatten() + .forEach { email -> + syncFlagsForMessage(backendFolder, syncConfig, email) + } + } + + private fun syncFlagsForMessage(backendFolder: BackendFolder, syncConfig: SyncConfig, email: Email) { + val messageServerId = email.id + val localFlags = backendFolder.getMessageFlags(messageServerId) + val remoteFlags = email.keywords.toFlags() + for (flag in syncConfig.syncFlags) { + val flagSetOnServer = flag in remoteFlags + val flagSetLocally = flag in localFlags + if (flagSetOnServer != flagSetLocally) { + backendFolder.setMessageFlag(messageServerId, flag, flagSetOnServer) + } + } + } + private fun Map?.toFlags(): Set { return if (this == null) { emptySet() @@ -260,6 +293,8 @@ class CommandSync( companion object { private const val EXTRA_QUERY_STATE = "jmapQueryState" private const val ERROR_CANNOT_CALCULATE_CHANGES = "cannotCalculateChanges" + private val INFO_PROPERTIES = arrayOf("id", "blobId", "size", "receivedAt", "keywords") + private val FLAG_PROPERTIES = arrayOf("id", "keywords") } } diff --git a/backend/jmap/src/main/java/com/fsck/k9/backend/jmap/JmapBackend.kt b/backend/jmap/src/main/java/com/fsck/k9/backend/jmap/JmapBackend.kt index c66eb20f1..1529565c8 100644 --- a/backend/jmap/src/main/java/com/fsck/k9/backend/jmap/JmapBackend.kt +++ b/backend/jmap/src/main/java/com/fsck/k9/backend/jmap/JmapBackend.kt @@ -43,7 +43,7 @@ class JmapBackend( } override fun sync(folder: String, syncConfig: SyncConfig, listener: SyncListener) { - commandSync.sync(folder, listener) + commandSync.sync(folder, syncConfig, listener) } override fun downloadMessage(syncConfig: SyncConfig, folderServerId: String, messageServerId: String) { diff --git a/backend/jmap/src/test/java/com/fsck/k9/backend/jmap/CommandSyncTest.kt b/backend/jmap/src/test/java/com/fsck/k9/backend/jmap/CommandSyncTest.kt index f28af70c1..2849b74bd 100644 --- a/backend/jmap/src/test/java/com/fsck/k9/backend/jmap/CommandSyncTest.kt +++ b/backend/jmap/src/test/java/com/fsck/k9/backend/jmap/CommandSyncTest.kt @@ -1,10 +1,14 @@ package com.fsck.k9.backend.jmap 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.mail.AuthenticationFailedException +import com.fsck.k9.mail.Flag import com.fsck.k9.mail.FolderType import com.fsck.k9.mail.internet.BinaryTempFileBody import java.io.File +import java.util.EnumSet import okhttp3.HttpUrl import okhttp3.OkHttpClient import okhttp3.mockwebserver.MockResponse @@ -19,6 +23,14 @@ class CommandSyncTest { private val backendStorage = InMemoryBackendStorage() private val okHttpClient = OkHttpClient.Builder().build() private val syncListener = LoggingSyncListener() + private val syncConfig = SyncConfig( + expungePolicy = ExpungePolicy.IMMEDIATELY, + earliestPollDate = null, + syncRemoteDeletions = true, + maximumAutoDownloadMessageSize = 1000, + defaultVisibleLimit = 25, + syncFlags = EnumSet.of(Flag.SEEN, Flag.FLAGGED, Flag.ANSWERED, Flag.FORWARDED) + ) @Before fun setUp() { @@ -32,7 +44,7 @@ class CommandSyncTest { MockResponse().setResponseCode(401) ) - command.sync(FOLDER_SERVER_ID, syncListener) + command.sync(FOLDER_SERVER_ID, syncConfig, syncListener) assertEquals(SyncListenerEvent.SyncStarted(FOLDER_SERVER_ID), syncListener.getNextEvent()) val failedEvent = syncListener.getNextEvent() as SyncListenerEvent.SyncFailed @@ -51,7 +63,7 @@ class CommandSyncTest { val baseUrl = server.url("/jmap/") val command = createCommandSync(baseUrl) - command.sync(FOLDER_SERVER_ID, syncListener) + command.sync(FOLDER_SERVER_ID, syncConfig, syncListener) val backendFolder = backendStorage.getFolder(FOLDER_SERVER_ID) backendFolder.assertMessages( @@ -85,10 +97,11 @@ class CommandSyncTest { responseBodyFromResource("/jmap_responses/blob/email/email_3.eml") ) - command.sync(FOLDER_SERVER_ID, syncListener) + command.sync(FOLDER_SERVER_ID, syncConfig, syncListener) val backendFolder = backendStorage.getFolder(FOLDER_SERVER_ID) assertEquals(setOf("M001", "M002", "M003", "M004", "M005"), backendFolder.getMessageServerIds()) + syncListener.assertSyncSuccess() } @Test @@ -102,15 +115,17 @@ class CommandSyncTest { responseBodyFromResource("/jmap_responses/session/valid_session.json"), responseBodyFromResource("/jmap_responses/email/email_query_M002_and_M003.json"), responseBodyFromResource("/jmap_responses/email/email_get_ids_M003.json"), - responseBodyFromResource("/jmap_responses/blob/email/email_3.eml") + responseBodyFromResource("/jmap_responses/blob/email/email_3.eml"), + responseBodyFromResource("/jmap_responses/email/email_get_keywords_M002.json") ) - command.sync(FOLDER_SERVER_ID, syncListener) + command.sync(FOLDER_SERVER_ID, syncConfig, syncListener) backendFolder.assertMessages( "M002" to "/jmap_responses/blob/email/email_2.eml", "M003" to "/jmap_responses/blob/email/email_3.eml" ) + syncListener.assertSyncSuccess() } @Test @@ -125,7 +140,7 @@ class CommandSyncTest { responseBodyFromResource("/jmap_responses/email/email_query_empty_result.json") ) - command.sync(FOLDER_SERVER_ID, syncListener) + command.sync(FOLDER_SERVER_ID, syncConfig, syncListener) assertEquals(emptySet(), backendFolder.getMessageServerIds()) syncListener.assertSyncEvents( @@ -144,12 +159,15 @@ class CommandSyncTest { backendFolder.setQueryState("50:0") val command = createCommandSync( responseBodyFromResource("/jmap_responses/session/valid_session.json"), - responseBodyFromResource("/jmap_responses/email/email_query_changes_empty_result.json") + responseBodyFromResource("/jmap_responses/email/email_query_changes_empty_result.json"), + responseBodyFromResource("/jmap_responses/email/email_get_keywords_M001_and_M002.json") ) - command.sync(FOLDER_SERVER_ID, syncListener) + command.sync(FOLDER_SERVER_ID, syncConfig, syncListener) assertEquals(setOf("M001", "M002"), backendFolder.getMessageServerIds()) + assertEquals(emptySet(), backendFolder.getMessageFlags("M001")) + assertEquals(setOf(Flag.SEEN), backendFolder.getMessageFlags("M002")) backendFolder.assertQueryState("50:0") syncListener.assertSyncEvents( SyncListenerEvent.SyncStarted(FOLDER_SERVER_ID), @@ -169,13 +187,15 @@ class CommandSyncTest { responseBodyFromResource("/jmap_responses/session/valid_session.json"), responseBodyFromResource("/jmap_responses/email/email_query_changes_M001_deleted_M003_added.json"), responseBodyFromResource("/jmap_responses/email/email_get_ids_M003.json"), - responseBodyFromResource("/jmap_responses/blob/email/email_3.eml") + responseBodyFromResource("/jmap_responses/blob/email/email_3.eml"), + responseBodyFromResource("/jmap_responses/email/email_get_keywords_M002.json") ) - command.sync(FOLDER_SERVER_ID, syncListener) + command.sync(FOLDER_SERVER_ID, syncConfig, syncListener) assertEquals(setOf("M002", "M003"), backendFolder.getMessageServerIds()) backendFolder.assertQueryState("51:0") + syncListener.assertSyncSuccess() } @Test @@ -191,13 +211,15 @@ class CommandSyncTest { responseBodyFromResource("/jmap_responses/email/email_query_changes_cannot_calculate_changes_error.json"), responseBodyFromResource("/jmap_responses/email/email_query_M002_and_M003.json"), responseBodyFromResource("/jmap_responses/email/email_get_ids_M003.json"), - responseBodyFromResource("/jmap_responses/blob/email/email_3.eml") + responseBodyFromResource("/jmap_responses/blob/email/email_3.eml"), + responseBodyFromResource("/jmap_responses/email/email_get_keywords_M002.json") ) - command.sync(FOLDER_SERVER_ID, syncListener) + command.sync(FOLDER_SERVER_ID, syncConfig, syncListener) assertEquals(setOf("M002", "M003"), backendFolder.getMessageServerIds()) backendFolder.assertQueryState("50:0") + syncListener.assertSyncSuccess() } private fun createCommandSync(vararg mockResponses: MockResponse): CommandSync { diff --git a/backend/jmap/src/test/java/com/fsck/k9/backend/jmap/InMemoryBackendFolder.kt b/backend/jmap/src/test/java/com/fsck/k9/backend/jmap/InMemoryBackendFolder.kt index ad80313d5..ab47590b8 100644 --- a/backend/jmap/src/test/java/com/fsck/k9/backend/jmap/InMemoryBackendFolder.kt +++ b/backend/jmap/src/test/java/com/fsck/k9/backend/jmap/InMemoryBackendFolder.kt @@ -13,6 +13,7 @@ class InMemoryBackendFolder(override var name: String, var type: FolderType) : B val extraStrings: MutableMap = mutableMapOf() val extraNumbers: MutableMap = mutableMapOf() private val messages = mutableMapOf() + private val messageFlags = mutableMapOf>() override var visibleLimit: Int = 25 @@ -39,6 +40,7 @@ class InMemoryBackendFolder(override var name: String, var type: FolderType) : B } messages[messageServerId] = message + messageFlags[messageServerId] = mutableSetOf() } } @@ -61,6 +63,7 @@ class InMemoryBackendFolder(override var name: String, var type: FolderType) : B override fun destroyMessages(messageServerIds: List) { for (messageServerId in messageServerIds) { messages.remove(messageServerId) + messageFlags.remove(messageServerId) } } @@ -97,21 +100,28 @@ class InMemoryBackendFolder(override var name: String, var type: FolderType) : B } override fun getMessageFlags(messageServerId: String): Set { - throw UnsupportedOperationException("not implemented") + return messageFlags[messageServerId] ?: error("Message $messageServerId not found") } override fun setMessageFlag(messageServerId: String, flag: Flag, value: Boolean) { - throw UnsupportedOperationException("not implemented") + val flags = messageFlags[messageServerId] ?: error("Message $messageServerId not found") + if (value) { + flags.add(flag) + } else { + flags.remove(flag) + } } override fun savePartialMessage(message: Message) { val messageServerId = checkNotNull(message.uid) messages[messageServerId] = message + messageFlags[messageServerId] = message.flags.toMutableSet() } override fun saveCompleteMessage(message: Message) { val messageServerId = checkNotNull(message.uid) messages[messageServerId] = message + messageFlags[messageServerId] = message.flags.toMutableSet() } override fun getLatestOldMessageSeenTime(): Date { diff --git a/backend/jmap/src/test/java/com/fsck/k9/backend/jmap/LoggingSyncListener.kt b/backend/jmap/src/test/java/com/fsck/k9/backend/jmap/LoggingSyncListener.kt index 0f6c17053..5918ea75c 100644 --- a/backend/jmap/src/test/java/com/fsck/k9/backend/jmap/LoggingSyncListener.kt +++ b/backend/jmap/src/test/java/com/fsck/k9/backend/jmap/LoggingSyncListener.kt @@ -1,12 +1,24 @@ package com.fsck.k9.backend.jmap import com.fsck.k9.backend.api.SyncListener +import java.lang.AssertionError import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue +import org.junit.Assert.fail class LoggingSyncListener : SyncListener { private val events = mutableListOf() + fun assertSyncSuccess() { + events.filterIsInstance().firstOrNull()?.let { syncFailed -> + throw AssertionError("Expected sync success", syncFailed.exception) + } + + if (events.none { it is SyncListenerEvent.SyncFinished }) { + fail("Expected SyncFinished, but only got: $events") + } + } + fun assertSyncEvents(vararg events: SyncListenerEvent) { for (event in events) { assertEquals(event, getNextEvent()) diff --git a/backend/jmap/src/test/resources/jmap_responses/email/email_get_keywords_M001_and_M002.json b/backend/jmap/src/test/resources/jmap_responses/email/email_get_keywords_M001_and_M002.json new file mode 100644 index 000000000..f7639744b --- /dev/null +++ b/backend/jmap/src/test/resources/jmap_responses/email/email_get_keywords_M001_and_M002.json @@ -0,0 +1,26 @@ +{ + "methodResponses": [ + [ + "Email/get", + { + "state": "50", + "list": [ + { + "id": "M001", + "keywords": {} + }, + { + "id": "M002", + "keywords": { + "$seen": true + } + } + ], + "notFound": [], + "accountId": "test@example.com" + }, + "0" + ] + ], + "sessionState": "0" +} diff --git a/backend/jmap/src/test/resources/jmap_responses/email/email_get_keywords_M002.json b/backend/jmap/src/test/resources/jmap_responses/email/email_get_keywords_M002.json new file mode 100644 index 000000000..2a1f08d07 --- /dev/null +++ b/backend/jmap/src/test/resources/jmap_responses/email/email_get_keywords_M002.json @@ -0,0 +1,20 @@ +{ + "methodResponses": [ + [ + "Email/get", + { + "state": "50", + "list": [ + { + "id": "M002", + "keywords": {} + } + ], + "notFound": [], + "accountId": "test@example.com" + }, + "0" + ] + ], + "sessionState": "0" +}