[Push] Don't enqueue sync if local collection is up-to-date

app/src/main/kotlin/at/bitfire/davdroid/db/SyncState.kt

@@ -9,16 +9,27 @@ import org.json.JSONException
 import org.json.JSONObject
 
 data class SyncState(
-        val type: Type,
-        val value: String,
-
-        /**
-         * Whether this sync state occurred during an initial sync as described
-         * in RFC 6578, which means the initial sync is not complete yet.
-         */
-        var initialSync: Boolean? = null
+    val type: Type,
+    val value: String,
+
+    /**
+     * Whether this sync state occurred during an initial sync as described
+     * in RFC 6578, which means the initial sync is not complete yet.
+     */
+    var initialSync: Boolean? = null
 ) {
 
+    enum class Type { CTAG, SYNC_TOKEN }
+
+    override fun toString(): String {
+        val json = JSONObject()
+        json.put(KEY_TYPE, type.name)
+        json.put(KEY_VALUE, value)
+        initialSync?.let { json.put(KEY_INITIAL_SYNC, it) }
+        return json.toString()
+    }
+
+
     companion object {
 
         private const val KEY_TYPE = "type"
@@ -32,28 +43,18 @@ data class SyncState(
             return try {
                 val json = JSONObject(s)
                 SyncState(
-                        Type.valueOf(json.getString(KEY_TYPE)),
-                        json.getString(KEY_VALUE),
-                        try { json.getBoolean(KEY_INITIAL_SYNC) } catch(e: JSONException) { null }
+                    Type.valueOf(json.getString(KEY_TYPE)),
+                    json.getString(KEY_VALUE),
+                    try { json.getBoolean(KEY_INITIAL_SYNC) } catch(e: JSONException) { null }
                 )
-            } catch (e: JSONException) {
+            } catch (_: JSONException) {
                 null
             }
         }
 
         fun fromSyncToken(token: SyncToken, initialSync: Boolean? = null) =
-                SyncState(Type.SYNC_TOKEN, requireNotNull(token.token), initialSync)
+            SyncState(Type.SYNC_TOKEN, requireNotNull(token.token), initialSync)
 
     }
 
-    enum class Type { CTAG, SYNC_TOKEN }
-
-    override fun toString(): String {
-        val json = JSONObject()
-        json.put(KEY_TYPE, type.name)
-        json.put(KEY_VALUE, value)
-        initialSync?.let { json.put(KEY_INITIAL_SYNC, it) }
-        return json.toString()
-    }
-
 }

app/src/main/kotlin/at/bitfire/davdroid/push/PushMessageParser.kt

@@ -8,6 +8,7 @@ import at.bitfire.dav4jvm.XmlReader
 import at.bitfire.dav4jvm.XmlUtils
 import at.bitfire.dav4jvm.property.push.PushMessage
 import at.bitfire.dav4jvm.property.push.Topic
+import at.bitfire.dav4jvm.property.webdav.SyncToken
 import org.xmlpull.v1.XmlPullParserException
 import java.io.StringReader
 import java.util.logging.Level
@@ -18,13 +19,19 @@ class PushMessageParser @Inject constructor(
     private val logger: Logger
 ) {
 
+    data class PushMessageBody(
+        val topic: String?,
+        val syncToken: String?
+    )
+
     /**
      * Parses a WebDAV-Push message and returns the `topic` that the message is about.
      *
      * @return topic of the modified collection, or `null` if the topic couldn't be determined
      */
-    operator fun invoke(message: String): String? {
+    operator fun invoke(message: String): PushMessageBody {
         var topic: String? = null
+        var syncToken: String? = null
 
         val parser = XmlUtils.newPullParser()
         try {
@@ -33,14 +40,17 @@ class PushMessageParser @Inject constructor(
             XmlReader(parser).processTag(PushMessage.NAME) {
                 val pushMessage = PushMessage.Factory.create(parser)
                 val properties = pushMessage.propStat?.properties ?: return@processTag
-                val pushTopic = properties.filterIsInstance<Topic>().firstOrNull()
-                topic = pushTopic?.topic
+                topic = properties.filterIsInstance<Topic>().firstOrNull()?.topic
+                syncToken = properties.filterIsInstance<SyncToken>().firstOrNull()?.token
             }
         } catch (e: XmlPullParserException) {
             logger.log(Level.WARNING, "Couldn't parse push message", e)
         }
 
-        return topic
+        return PushMessageBody(
+            topic = topic,
+            syncToken = syncToken
+        )
     }
 
 }

app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushReceiver.kt

@@ -4,12 +4,17 @@
 
 package at.bitfire.davdroid.push
 
+import android.accounts.Account
 import android.content.Context
 import at.bitfire.davdroid.db.Collection.Companion.TYPE_ADDRESSBOOK
+import at.bitfire.davdroid.db.SyncState
 import at.bitfire.davdroid.repository.AccountRepository
 import at.bitfire.davdroid.repository.DavCollectionRepository
 import at.bitfire.davdroid.repository.DavServiceRepository
 import at.bitfire.davdroid.repository.PreferenceRepository
+import at.bitfire.davdroid.resource.LocalAddressBookStore
+import at.bitfire.davdroid.resource.LocalCalendarStore
+import at.bitfire.davdroid.resource.LocalDataStore
 import at.bitfire.davdroid.sync.SyncDataType
 import at.bitfire.davdroid.sync.TasksAppManager
 import at.bitfire.davdroid.sync.worker.SyncWorkerManager
@@ -31,7 +36,13 @@ class UnifiedPushReceiver: MessagingReceiver() {
 
     @Inject
     lateinit var collectionRepository: DavCollectionRepository
-
+
+    @Inject
+    lateinit var localAddressBookStore: Lazy<LocalAddressBookStore>
+
+    @Inject
+    lateinit var localCalendarStore: Lazy<LocalCalendarStore>
+
     @Inject
     lateinit var logger: Logger
 
@@ -48,10 +59,10 @@ class UnifiedPushReceiver: MessagingReceiver() {
     lateinit var pushRegistrationWorkerManager: PushRegistrationWorkerManager
 
     @Inject
-    lateinit var tasksAppManager: Lazy<TasksAppManager>
+    lateinit var syncWorkerManager: SyncWorkerManager
 
     @Inject
-    lateinit var syncWorkerManager: SyncWorkerManager
+    lateinit var tasksAppManager: Lazy<TasksAppManager>
 
 
     override fun onNewEndpoint(context: Context, endpoint: String, instance: String) {
@@ -73,15 +84,15 @@ class UnifiedPushReceiver: MessagingReceiver() {
             logger.log(Level.INFO, "Received push message", messageXml)
 
             // parse push notification
-            val topic = parsePushMessage(messageXml)
+            val pushMessage = parsePushMessage(messageXml)
 
             // sync affected collection
-            if (topic != null) {
-                logger.info("Got push notification for topic $topic")
+            if (pushMessage.topic != null) {
+                logger.info("Got push notification for topic ${pushMessage.topic} with sync-token=${pushMessage.syncToken}")
 
                 // Sync all authorities of account that the collection belongs to
                 // Later: only sync affected collection and authorities
-                collectionRepository.getSyncableByTopic(topic)?.let { collection ->
+                collectionRepository.getSyncableByTopic(pushMessage.topic)?.let { collection ->
                     serviceRepository.get(collection.serviceId)?.let { service ->
                         val syncDataTypes = mutableSetOf<SyncDataType>()
                         // If the type is an address book, add the contacts type
@@ -100,18 +111,40 @@ class UnifiedPushReceiver: MessagingReceiver() {
 
                         // Schedule sync for all the types identified
                         val account = accountRepository.fromName(service.accountName)
-                        for (syncDataType in syncDataTypes)
-                            syncWorkerManager.enqueueOneTime(account, syncDataType, fromPush = true)
+                        for (syncDataType in syncDataTypes) {
+                            val localSyncToken = lastSyncToken(account, syncDataType, collection.id)
+                            if (localSyncToken == pushMessage.syncToken)
+                                logger.fine("Collection $collection already up-to-date ($localSyncToken), ignoring push message")
+                            else
+                                syncWorkerManager.enqueueOneTime(account, syncDataType, fromPush = true)
+                        }
                     }
                 }
 
             } else {
                 logger.warning("Got push message without topic, syncing all accounts")
                 for (account in accountRepository.getAll())
-                    syncWorkerManager.enqueueOneTimeAllAuthorities(account, fromPush = true)
+                    syncWorkerManager.enqueueOneTimeAllAuthorities(account = account, fromPush = true)
+
+            }
+        }
+    }
+
+    fun lastSyncToken(account: Account, dataType: SyncDataType, collectionId: Long): String? {
+        val dataStore: LocalDataStore<*>? = when (dataType) {
+            SyncDataType.CONTACTS -> localAddressBookStore.get()
+            SyncDataType.EVENTS -> localCalendarStore.get()
+            SyncDataType.TASKS -> tasksAppManager.get().getDataStore()
+        }
 
+        dataStore?.acquireContentProvider()?.use { provider ->
+            val localCollection = dataStore.getByLocalId(account, provider, collectionId)
+            return localCollection?.lastSyncState?.value.takeIf {
+                localCollection?.lastSyncState?.type == SyncState.Type.SYNC_TOKEN
             }
         }
+
+        return null
     }
 
 }

app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBookStore.kt

@@ -123,6 +123,17 @@ class LocalAddressBookStore @Inject constructor(
             }
     }
 
+    override fun getByLocalId(account: Account, provider: ContentProviderClient, id: Long): LocalAddressBook? {
+        val accountManager = AccountManager.get(context)
+        return accountManager.getAccountsByType(context.getString(R.string.account_type_address_book))
+            .filter { addressBookAccount ->
+                accountManager.getUserData(addressBookAccount, LocalAddressBook.USER_DATA_COLLECTION_ID).toLongOrNull() == id
+            }
+            .map { addressBookAccount ->
+                localAddressBookFactory.create(account, addressBookAccount, provider)
+            }.firstOrNull()
+    }
+
     override fun update(provider: ContentProviderClient, localCollection: LocalAddressBook, fromCollection: Collection) {
         var currentAccount = localCollection.addressBookAccount
         logger.log(Level.INFO, "Updating local address book $currentAccount from collection $fromCollection")

app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCalendarStore.kt

@@ -76,6 +76,9 @@ class LocalCalendarStore @Inject constructor(
     override fun getAll(account: Account, provider: ContentProviderClient) =
         AndroidCalendar.find(account, provider, LocalCalendar.Factory, "${Calendars.SYNC_EVENTS}!=0", null)
 
+    override fun getByLocalId(account: Account, provider: ContentProviderClient, id: Long): LocalCalendar? =
+        AndroidCalendar.find(account, provider, LocalCalendar.Factory, "${Calendars._SYNC_ID}=?", arrayOf(id.toString())).firstOrNull()
+
     override fun update(provider: ContentProviderClient, localCollection: LocalCalendar, fromCollection: Collection) {
         val accountSettings = accountSettingsFactory.create(localCollection.account)
         val values = valuesFromCollectionInfo(fromCollection, withColor = accountSettings.getManageCalendarColors())
@@ -123,4 +126,4 @@ class LocalCalendarStore @Inject constructor(
         localCollection.delete()
     }
 
-}
+}

app/src/main/kotlin/at/bitfire/davdroid/resource/LocalDataStore.kt

@@ -52,6 +52,17 @@ interface LocalDataStore<T: LocalCollection<*>> {
      */
     fun getAll(account: Account, provider: ContentProviderClient): List<T>
 
+    /**
+     * Retrieves the local collection with the given local (database) ID.
+     *
+     * @param account  the account that the data store is associated with
+     * @param provider the content provider client
+     * @param id        the local (database) ID of the collection ([Collection.id])
+     *
+     * @return the local collection, or `null` if none was found
+     */
+    fun getByLocalId(account: Account, provider: ContentProviderClient, id: Long): T?
+
     /**
      * Updates the local collection with the data from the given (remote) collection info.
      *
@@ -61,13 +72,6 @@ interface LocalDataStore<T: LocalCollection<*>> {
      */
     fun update(provider: ContentProviderClient, localCollection: T, fromCollection: Collection)
 
-    /**
-     * Deletes the local collection.
-     *
-     * @param localCollection the local collection to delete
-     */
-    fun delete(localCollection: T)
-
     /**
      * Changes the account assigned to the containing data to another one.
      *
@@ -76,4 +80,11 @@ interface LocalDataStore<T: LocalCollection<*>> {
      */
     fun updateAccount(oldAccount: Account, newAccount: Account)
 
+    /**
+     * Deletes the local collection.
+     *
+     * @param localCollection the local collection to delete
+     */
+    fun delete(localCollection: T)
+
 }

app/src/main/kotlin/at/bitfire/davdroid/resource/LocalJtxCollectionStore.kt

@@ -91,6 +91,9 @@ class LocalJtxCollectionStore @Inject constructor(
     override fun getAll(account: Account, provider: ContentProviderClient): List<LocalJtxCollection> =
         JtxCollection.find(account, provider, context, LocalJtxCollection.Factory, null, null)
 
+    override fun getByLocalId(account: Account, provider: ContentProviderClient, id: Long): LocalJtxCollection? =
+        JtxCollection.find(account, provider, context, LocalJtxCollection.Factory, "${JtxContract.JtxCollection.SYNC_ID}=?", arrayOf(id.toString())).firstOrNull()
+
     override fun update(provider: ContentProviderClient, localCollection: LocalJtxCollection, fromCollection: Collection) {
         val accountSettings = accountSettingsFactory.create(localCollection.account)
         val values = valuesFromCollection(fromCollection, account = localCollection.account, withColor = accountSettings.getManageCalendarColors())

app/src/main/kotlin/at/bitfire/davdroid/resource/LocalTaskListStore.kt

@@ -97,6 +97,9 @@ class LocalTaskListStore @AssistedInject constructor(
     override fun getAll(account: Account, provider: ContentProviderClient) =
         DmfsTaskList.find(account, LocalTaskList.Factory, provider, providerName, null, null)
 
+    override fun getByLocalId(account: Account, provider: ContentProviderClient, id: Long): LocalTaskList? =
+        DmfsTaskList.find(account, LocalTaskList.Factory, provider, providerName, "${TaskLists._SYNC_ID}=?", arrayOf(id.toString())).firstOrNull()
+
     override fun update(provider: ContentProviderClient, localCollection: LocalTaskList, fromCollection: Collection) {
         logger.log(Level.FINE, "Updating local task list ${fromCollection.url}", fromCollection)
         val accountSettings = accountSettingsFactory.create(localCollection.account)