Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Draft
wants to merge 1 commit into
base: main-ose
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 24 additions & 23 deletions app/src/main/kotlin/at/bitfire/davdroid/db/SyncState.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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()
}

}
18 changes: 14 additions & 4 deletions app/src/main/kotlin/at/bitfire/davdroid/push/PushMessageParser.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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
)
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
package at.bitfire.davdroid.push

import android.content.Context
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.LocalDataStore
import at.bitfire.davdroid.sync.worker.SyncWorkerManager
import dagger.hilt.android.AndroidEntryPoint
import kotlinx.coroutines.CoroutineScope
Expand Down Expand Up @@ -66,25 +68,39 @@ 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 account = accountRepository.fromName(service.accountName)
syncWorkerManager.enqueueOneTimeAllAuthorities(account, fromPush = true)
for (authority in syncWorkerManager.syncAuthorities()) {
// TODO: get LocalDataStore according to authority and service (i.e. LocalAddressBookStore if authority is contacts, etc.)
val dataStore: LocalDataStore<*>? = TODO()

val localCollection = dataStore?.getByLocalId(collection.id)
val lastSyncedSyncToken = localCollection?.lastSyncState?.value.takeIf {
localCollection?.lastSyncState?.type == SyncState.Type.SYNC_TOKEN
}
if (lastSyncedSyncToken == pushMessage.syncToken) {
logger.fine("Collection $collection is already up-to-date, ignoring push message")
continue
}

val account = accountRepository.fromName(service.accountName)
syncWorkerManager.enqueueOneTime(account = account, authority = authority, 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)

}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ interface LocalDataStore<T: LocalCollection<*>> {
fun create(provider: ContentProviderClient, fromCollection: Collection): T?

/**
* Returns all local collections of the data store.
* Retrieves all local collections of the data store.
*
* @param account the account that the data store is associated with
* @param provider the content provider client
Expand All @@ -34,6 +34,15 @@ interface LocalDataStore<T: LocalCollection<*>> {
*/
fun getAll(account: Account, provider: ContentProviderClient): List<T>

/**
* Retrieves the local collection with the given local (database) ID.
*
* @param id the local (database) ID of the collection ([Collection.id])
*
* @return the local collection, or `null` if none was found
*/
fun getByLocalId(id: Long): T?

/**
* Updates the local collection with the data from the given (remote) collection info.
*
Expand Down
Loading