From 9d4f747c123f1edd2de74d10b1ce83ceccbf8110 Mon Sep 17 00:00:00 2001 From: Arnau Mora Date: Tue, 25 Feb 2025 11:38:51 +0100 Subject: [PATCH 01/20] Upgrade UnifiedPush Connector to 3.0.4 Signed-off-by: Arnau Mora --- gradle/libs.versions.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index dad9321fce..eedc49f91f 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -39,7 +39,7 @@ mockk = "1.13.16" okhttp = "4.12.0" openid-appauth = "0.11.1" room = "2.6.1" -unifiedpush = "2.4.0" +unifiedpush = "3.0.4" [libraries] android-desugaring = { module = "com.android.tools:desugar_jdk_libs_nio", version.ref = "android-desugaring" } From bd1f2ae1f742ebf195319c47689e1ef0eeb3556d Mon Sep 17 00:00:00 2001 From: Arnau Mora Date: Tue, 25 Feb 2025 11:39:56 +0100 Subject: [PATCH 02/20] Updated overrides Signed-off-by: Arnau Mora --- .../davdroid/push/UnifiedPushReceiver.kt | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushReceiver.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushReceiver.kt index be94a03a5b..87aa650391 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushReceiver.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushReceiver.kt @@ -18,7 +18,10 @@ import dagger.hilt.android.AndroidEntryPoint import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch +import org.unifiedpush.android.connector.FailedReason import org.unifiedpush.android.connector.MessagingReceiver +import org.unifiedpush.android.connector.data.PushEndpoint +import org.unifiedpush.android.connector.data.PushMessage import java.util.logging.Level import java.util.logging.Logger import javax.inject.Inject @@ -54,7 +57,7 @@ class UnifiedPushReceiver: MessagingReceiver() { lateinit var syncWorkerManager: SyncWorkerManager - override fun onNewEndpoint(context: Context, endpoint: String, instance: String) { + override fun onNewEndpoint(context: Context, endpoint: PushEndpoint, instance: String) { // remember new endpoint preferenceRepository.unifiedPushEndpoint(endpoint) @@ -62,14 +65,24 @@ class UnifiedPushReceiver: MessagingReceiver() { pushRegistrationWorkerManager.updatePeriodicWorker() } + override fun onRegistrationFailed(context: Context, reason: FailedReason, instance: String) { + logger.warning("Unified Push registration failed: $reason") + // reset known endpoint to make sure nothing is stored when not registered + preferenceRepository.unifiedPushEndpoint(null) + } + override fun onUnregistered(context: Context, instance: String) { // reset known endpoint preferenceRepository.unifiedPushEndpoint(null) } - override fun onMessage(context: Context, message: ByteArray, instance: String) { + override fun onMessage(context: Context, message: PushMessage, instance: String) { CoroutineScope(Dispatchers.Default).launch { - val messageXml = message.toString(Charsets.UTF_8) + if (!message.decrypted) { + logger.severe("Received a push message that could not be decrypted.") + return@launch + } + val messageXml = message.content.toString(Charsets.UTF_8) logger.log(Level.INFO, "Received push message", messageXml) // parse push notification From 24a31a3a092cd237ae478a3da18031b86f2aa5ef Mon Sep 17 00:00:00 2001 From: Arnau Mora Date: Tue, 25 Feb 2025 11:40:34 +0100 Subject: [PATCH 03/20] Added storing keys and auths Signed-off-by: Arnau Mora --- .../davdroid/push/PushRegistrationWorker.kt | 16 +++++++++++-- .../repository/PreferenceRepository.kt | 23 ++++++++++++++----- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt index 06d311a838..816e76da79 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt @@ -29,6 +29,7 @@ import kotlinx.coroutines.runInterruptible import okhttp3.HttpUrl import okhttp3.HttpUrl.Companion.toHttpUrlOrNull import okhttp3.RequestBody.Companion.toRequestBody +import org.unifiedpush.android.connector.data.PushEndpoint import java.io.IOException import java.io.StringWriter import java.time.Duration @@ -67,7 +68,7 @@ class PushRegistrationWorker @AssistedInject constructor( return Result.success() } - private suspend fun registerPushSubscription(collection: Collection, account: Account, endpoint: String) { + private suspend fun registerPushSubscription(collection: Collection, account: Account, endpoint: PushEndpoint) { httpClientBuilder.get() .fromAccount(account) .inForeground(true) @@ -88,7 +89,18 @@ class PushRegistrationWorker @AssistedInject constructor( // subscription URL serializer.insertTag(Property.Name(NS_WEBDAV_PUSH, "web-push-subscription")) { serializer.insertTag(Property.Name(NS_WEBDAV_PUSH, "push-resource")) { - text(endpoint) + text(endpoint.url) + } + endpoint.pubKeySet?.let { pubKeySet -> + // Right now only p256dh is supported, but more can be added in + // the future. + serializer.insertTag(Property.Name(NS_WEBDAV_PUSH, "client-public-key")) { + attribute(NS_WEBDAV_PUSH, "type", "p256dh") + text(pubKeySet.pubKey) + } + serializer.insertTag(Property.Name(NS_WEBDAV_PUSH, "auth-secret")) { + text(pubKeySet.auth) + } } } } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/PreferenceRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/PreferenceRepository.kt index cb26149c48..0314a14a43 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/repository/PreferenceRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/repository/PreferenceRepository.kt @@ -11,6 +11,8 @@ import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.callbackFlow +import org.unifiedpush.android.connector.data.PublicKeySet +import org.unifiedpush.android.connector.data.PushEndpoint import javax.inject.Inject /** @@ -24,7 +26,9 @@ class PreferenceRepository @Inject constructor( companion object { const val LOG_TO_FILE = "log_to_file" - const val UNIFIED_PUSH_ENDPOINT = "unified_push_endpoint" + const val UNIFIED_PUSH_ENDPOINT_URL = "unified_push_endpoint_url" + const val UNIFIED_PUSH_ENDPOINT_KEY = "unified_push_endpoint_key" + const val UNIFIED_PUSH_ENDPOINT_AUTH = "unified_push_endpoint_auth" } private val preferences = PreferenceManager.getDefaultSharedPreferences(context) @@ -54,17 +58,24 @@ class PreferenceRepository @Inject constructor( } - fun unifiedPushEndpoint() = - preferences.getString(UNIFIED_PUSH_ENDPOINT, null) + fun unifiedPushEndpoint(): PushEndpoint? { + val url = preferences.getString(UNIFIED_PUSH_ENDPOINT_URL, null) ?: return null + val key = preferences.getString(UNIFIED_PUSH_ENDPOINT_KEY, null) + val auth = preferences.getString(UNIFIED_PUSH_ENDPOINT_AUTH, null) + val publicKeySet = if (key != null && auth != null) PublicKeySet(key, auth) else null + return PushEndpoint(url, publicKeySet) + } - fun unifiedPushEndpointFlow() = observeAsFlow(UNIFIED_PUSH_ENDPOINT) { + fun unifiedPushEndpointFlow() = observeAsFlow(UNIFIED_PUSH_ENDPOINT_URL) { unifiedPushEndpoint() } - fun unifiedPushEndpoint(endpoint: String?) { + fun unifiedPushEndpoint(endpoint: PushEndpoint?) { preferences .edit() - .putString(UNIFIED_PUSH_ENDPOINT, endpoint) + .putString(UNIFIED_PUSH_ENDPOINT_URL, endpoint?.url) + .putString(UNIFIED_PUSH_ENDPOINT_KEY, endpoint?.pubKeySet?.pubKey) + .putString(UNIFIED_PUSH_ENDPOINT_AUTH, endpoint?.pubKeySet?.auth) .apply() } From bab946361b74c30d8b61911958df174be8b449dd Mon Sep 17 00:00:00 2001 From: Arnau Mora Date: Tue, 25 Feb 2025 11:58:29 +0100 Subject: [PATCH 04/20] Excluded tink Signed-off-by: Arnau Mora --- app/build.gradle.kts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/build.gradle.kts b/app/build.gradle.kts index b3e75600d6..23ae11cdfe 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -195,7 +195,12 @@ dependencies { implementation(libs.okhttp.brotli) implementation(libs.okhttp.logging) implementation(libs.openid.appauth) - implementation(libs.unifiedpush) + implementation(libs.unifiedpush) { + // UnifiedPush connector seems to be using a workaround by importing this library. + // Will be removed after https://github.com/tink-crypto/tink-java-apps/pull/5 is merged. + // See: https://codeberg.org/UnifiedPush/android-connector/src/commit/28cb0d622ed0a972996041ab9cc85b701abc48c6/connector/build.gradle#L56-L59 + exclude(group = "com.google.crypto.tink", module = "tink") + } // for tests androidTestImplementation(libs.androidx.arch.core.testing) From f961096543f7e5e6af583887f5a021689d73a214 Mon Sep 17 00:00:00 2001 From: Arnau Mora Date: Tue, 25 Feb 2025 11:58:40 +0100 Subject: [PATCH 05/20] Fixed deprecations and calls Signed-off-by: Arnau Mora --- .../main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt index 73551c2e65..e2c7916a41 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt @@ -160,12 +160,12 @@ class AppSettingsModel @Inject constructor( viewModelScope.launch(Dispatchers.IO) { if (pushDistributor == null) { // Disable UnifiedPush if the distributor given is null - UnifiedPush.safeRemoveDistributor(context) - UnifiedPush.unregisterApp(context) + UnifiedPush.removeDistributor(context) + UnifiedPush.unregister(context) } else { // If a distributor was passed, store it and register the app UnifiedPush.saveDistributor(context, pushDistributor) - UnifiedPush.registerApp(context) + UnifiedPush.register(context) } _pushDistributor.value = pushDistributor } From 07b1eea77fc7be14f374b8853780fa1f82c85fc2 Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Sun, 13 Apr 2025 15:57:34 +0200 Subject: [PATCH 06/20] Integrate UnifiedPush 3.x connector and FCM distributor --- app/build.gradle.kts | 1 + app/src/main/AndroidManifest.xml | 9 +++------ .../at/bitfire/davdroid/push/UnifiedPushReceiver.kt | 13 ++++++------- .../at/bitfire/davdroid/ui/AppSettingsModel.kt | 2 +- gradle/libs.versions.toml | 4 +++- 5 files changed, 14 insertions(+), 15 deletions(-) diff --git a/app/build.gradle.kts b/app/build.gradle.kts index df8858a565..c35817cf5d 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -201,6 +201,7 @@ dependencies { // See: https://codeberg.org/UnifiedPush/android-connector/src/commit/28cb0d622ed0a972996041ab9cc85b701abc48c6/connector/build.gradle#L56-L59 exclude(group = "com.google.crypto.tink", module = "tink") } + implementation(libs.unifiedpush.fcm) // for tests androidTestImplementation(libs.androidx.arch.core.testing) diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 173b80fc00..82ece5f46b 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -268,13 +268,10 @@ android:resource="@xml/debug_paths" /> - - + + - - - - + diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushReceiver.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushReceiver.kt index 87aa650391..98f5d26585 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushReceiver.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushReceiver.kt @@ -4,7 +4,6 @@ package at.bitfire.davdroid.push -import android.content.Context import at.bitfire.davdroid.db.Collection.Companion.TYPE_ADDRESSBOOK import at.bitfire.davdroid.repository.AccountRepository import at.bitfire.davdroid.repository.DavCollectionRepository @@ -19,7 +18,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import org.unifiedpush.android.connector.FailedReason -import org.unifiedpush.android.connector.MessagingReceiver +import org.unifiedpush.android.connector.PushService import org.unifiedpush.android.connector.data.PushEndpoint import org.unifiedpush.android.connector.data.PushMessage import java.util.logging.Level @@ -27,7 +26,7 @@ import java.util.logging.Logger import javax.inject.Inject @AndroidEntryPoint -class UnifiedPushReceiver: MessagingReceiver() { +class UnifiedPushReceiver: PushService() { @Inject lateinit var accountRepository: AccountRepository @@ -57,7 +56,7 @@ class UnifiedPushReceiver: MessagingReceiver() { lateinit var syncWorkerManager: SyncWorkerManager - override fun onNewEndpoint(context: Context, endpoint: PushEndpoint, instance: String) { + override fun onNewEndpoint(endpoint: PushEndpoint, instance: String) { // remember new endpoint preferenceRepository.unifiedPushEndpoint(endpoint) @@ -65,18 +64,18 @@ class UnifiedPushReceiver: MessagingReceiver() { pushRegistrationWorkerManager.updatePeriodicWorker() } - override fun onRegistrationFailed(context: Context, reason: FailedReason, instance: String) { + override fun onRegistrationFailed(reason: FailedReason, instance: String) { logger.warning("Unified Push registration failed: $reason") // reset known endpoint to make sure nothing is stored when not registered preferenceRepository.unifiedPushEndpoint(null) } - override fun onUnregistered(context: Context, instance: String) { + override fun onUnregistered(instance: String) { // reset known endpoint preferenceRepository.unifiedPushEndpoint(null) } - override fun onMessage(context: Context, message: PushMessage, instance: String) { + override fun onMessage(message: PushMessage, instance: String) { CoroutineScope(Dispatchers.Default).launch { if (!message.decrypted) { logger.severe("Received a push message that could not be decrypted.") diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt index e2c7916a41..162c67afb8 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt @@ -130,7 +130,7 @@ class AppSettingsModel @Inject constructor( * - If there's only one push distributor available, and none is selected, it's selected automatically. * - Makes sure the app is registered with UnifiedPush if there's already a distributor selected. */ - private suspend fun loadPushDistributors() { + private fun loadPushDistributors() { val savedPushDistributor = UnifiedPush.getSavedDistributor(context) _pushDistributor.value = savedPushDistributor diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 2df5b8c758..0435fdb763 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -40,6 +40,7 @@ okhttp = "4.12.0" openid-appauth = "0.11.1" room = "2.7.0" unifiedpush = "3.0.7" +unifiedpush-fcm = "3.0.0" [libraries] android-desugaring = { module = "com.android.tools:desugar_jdk_libs_nio", version.ref = "android-desugaring" } @@ -99,7 +100,8 @@ room-compiler = { module = "androidx.room:room-compiler", version.ref = "room" } room-paging = { module = "androidx.room:room-paging", version.ref = "room" } room-runtime = { module = "androidx.room:room-runtime", version.ref = "room" } room-testing = { module = "androidx.room:room-testing", version.ref = "room" } -unifiedpush = { module = "com.github.UnifiedPush:android-connector", version.ref = "unifiedpush" } +unifiedpush = { module = "org.unifiedpush.android:connector", version.ref = "unifiedpush" } +unifiedpush-fcm = { module = "org.unifiedpush.android:embedded-fcm-distributor", version.ref = "unifiedpush-fcm" } [plugins] android-application = { id = "com.android.application", version.ref = "android-agp" } From c6a4de7c3b86cd62e835d306d7ff78ba9de8ec4b Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Sun, 13 Apr 2025 17:13:42 +0200 Subject: [PATCH 07/20] Integrate UnifiedPush connector 3.x, use VAPID and message encryption --- app/src/main/AndroidManifest.xml | 4 +-- .../at/bitfire/davdroid/db/Collection.kt | 1 + .../at/bitfire/davdroid/db/CollectionDao.kt | 3 ++ .../davdroid/push/PushRegistrationWorker.kt | 33 ++++++++++--------- ...dPushReceiver.kt => UnifiedPushService.kt} | 7 ++-- .../repository/DavCollectionRepository.kt | 8 ++--- .../repository/PreferenceRepository.kt | 23 +++++-------- .../bitfire/davdroid/ui/AppSettingsModel.kt | 20 ++++++++--- 8 files changed, 57 insertions(+), 42 deletions(-) rename app/src/main/kotlin/at/bitfire/davdroid/push/{UnifiedPushReceiver.kt => UnifiedPushService.kt} (95%) diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 82ece5f46b..c73e4078a9 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -269,11 +269,11 @@ - + - + + @Query("SELECT COUNT(*) FROM collection WHERE serviceId=:serviceId AND type=:type") suspend fun anyOfType(serviceId: Long, @CollectionType type: String): Boolean diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt index 35011e182d..29df297146 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt @@ -12,11 +12,15 @@ import androidx.work.WorkerParameters import at.bitfire.dav4jvm.DavCollection import at.bitfire.dav4jvm.DavResource import at.bitfire.dav4jvm.HttpUtils -import at.bitfire.dav4jvm.Property import at.bitfire.dav4jvm.XmlUtils import at.bitfire.dav4jvm.XmlUtils.insertTag import at.bitfire.dav4jvm.exception.DavException -import at.bitfire.dav4jvm.property.push.NS_WEBDAV_PUSH +import at.bitfire.dav4jvm.property.push.AuthSecret +import at.bitfire.dav4jvm.property.push.PushRegister +import at.bitfire.dav4jvm.property.push.PushResource +import at.bitfire.dav4jvm.property.push.Subscription +import at.bitfire.dav4jvm.property.push.SubscriptionPublicKey +import at.bitfire.dav4jvm.property.push.WebPushSubscription import at.bitfire.davdroid.R import at.bitfire.davdroid.db.Collection import at.bitfire.davdroid.network.HttpClient @@ -83,28 +87,26 @@ class PushRegistrationWorker @AssistedInject constructor( val writer = StringWriter() serializer.setOutput(writer) serializer.startDocument("UTF-8", true) - serializer.insertTag(Property.Name(NS_WEBDAV_PUSH, "push-register")) { - serializer.insertTag(Property.Name(NS_WEBDAV_PUSH, "subscription")) { + serializer.insertTag(PushRegister.NAME) { + serializer.insertTag(Subscription.NAME) { // subscription URL - serializer.insertTag(Property.Name(NS_WEBDAV_PUSH, "web-push-subscription")) { - serializer.insertTag(Property.Name(NS_WEBDAV_PUSH, "push-resource")) { + serializer.insertTag(WebPushSubscription.NAME) { + serializer.insertTag(PushResource.NAME) { text(endpoint.url) } endpoint.pubKeySet?.let { pubKeySet -> - // Right now only p256dh is supported, but more can be added in - // the future. - serializer.insertTag(Property.Name(NS_WEBDAV_PUSH, "client-public-key")) { - attribute(NS_WEBDAV_PUSH, "type", "p256dh") + serializer.insertTag(SubscriptionPublicKey.NAME) { + attribute(null, "type", "p256dh") text(pubKeySet.pubKey) } - serializer.insertTag(Property.Name(NS_WEBDAV_PUSH, "auth-secret")) { + serializer.insertTag(AuthSecret.NAME) { text(pubKeySet.auth) } } } } // requested expiration - serializer.insertTag(Property.Name(NS_WEBDAV_PUSH, "expires")) { + serializer.insertTag(PushRegister.EXPIRES) { text(HttpUtils.formatDate(requestedExpiration)) } } @@ -133,11 +135,12 @@ class PushRegistrationWorker @AssistedInject constructor( val endpoint = preferenceRepository.unifiedPushEndpoint() // register push subscription for syncable collections - if (endpoint != null) + if (endpoint != null) { + // subscribe to collections for (collection in collectionRepository.getPushCapableAndSyncable()) { val expires = collection.pushSubscriptionExpires // calculate next run time, but use the duplicate interval for safety (times are not exact) - val nextRun = Instant.now() + Duration.ofDays(2*PushRegistrationWorkerManager.INTERVAL_DAYS) + val nextRun = Instant.now() + Duration.ofDays(2 * PushRegistrationWorkerManager.INTERVAL_DAYS) if (expires != null && expires >= nextRun.epochSecond) { logger.fine("Push subscription for ${collection.url} is still valid until ${collection.pushSubscriptionExpires}") continue @@ -155,7 +158,7 @@ class PushRegistrationWorker @AssistedInject constructor( } } } - else + } else logger.info("No UnifiedPush endpoint configured") } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushReceiver.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt similarity index 95% rename from app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushReceiver.kt rename to app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt index 98f5d26585..34e37cd467 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushReceiver.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt @@ -26,7 +26,7 @@ import java.util.logging.Logger import javax.inject.Inject @AndroidEntryPoint -class UnifiedPushReceiver: PushService() { +class UnifiedPushService: PushService() { @Inject lateinit var accountRepository: AccountRepository @@ -57,6 +57,8 @@ class UnifiedPushReceiver: PushService() { override fun onNewEndpoint(endpoint: PushEndpoint, instance: String) { + logger.info("Got UnifiedPush endpoint: ${endpoint.url}") + // remember new endpoint preferenceRepository.unifiedPushEndpoint(endpoint) @@ -65,12 +67,13 @@ class UnifiedPushReceiver: PushService() { } override fun onRegistrationFailed(reason: FailedReason, instance: String) { - logger.warning("Unified Push registration failed: $reason") + logger.warning("UnifiedPush registration failed: $reason") // reset known endpoint to make sure nothing is stored when not registered preferenceRepository.unifiedPushEndpoint(null) } override fun onUnregistered(instance: String) { + logger.warning("UnifiedPush unregistered") // reset known endpoint preferenceRepository.unifiedPushEndpoint(null) } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt index f483e3fb84..fe9834a47a 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt @@ -212,11 +212,11 @@ class DavCollectionRepository @Inject constructor( fun getSyncTaskLists(serviceId: Long) = dao.getSyncTaskLists(serviceId) /** Returns all collections that are both selected for synchronization and push-capable. */ - suspend fun getPushCapableAndSyncable(): List = - dao.getPushCapableSyncCollections() + suspend fun getPushCapableAndSyncable() = dao.getPushCapableSyncCollections() - suspend fun getPushRegisteredAndNotSyncable(): List = - dao.getPushRegisteredAndNotSyncable() + suspend fun getPushRegisteredAndNotSyncable() = dao.getPushRegisteredAndNotSyncable() + + suspend fun getVapidKeys() = dao.getVapidKeys() /** * Inserts or updates the collection. diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/PreferenceRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/PreferenceRepository.kt index 0314a14a43..e3bc718e30 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/repository/PreferenceRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/repository/PreferenceRepository.kt @@ -6,6 +6,7 @@ package at.bitfire.davdroid.repository import android.content.Context import android.content.SharedPreferences.OnSharedPreferenceChangeListener +import androidx.core.content.edit import androidx.preference.PreferenceManager import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.channels.awaitClose @@ -38,10 +39,9 @@ class PreferenceRepository @Inject constructor( * Updates the "log to file" (verbose logging") preference. */ fun logToFile(logToFile: Boolean) { - preferences - .edit() - .putBoolean(LOG_TO_FILE, logToFile) - .apply() + preferences.edit { + putBoolean(LOG_TO_FILE, logToFile) + } } /** @@ -66,17 +66,12 @@ class PreferenceRepository @Inject constructor( return PushEndpoint(url, publicKeySet) } - fun unifiedPushEndpointFlow() = observeAsFlow(UNIFIED_PUSH_ENDPOINT_URL) { - unifiedPushEndpoint() - } - fun unifiedPushEndpoint(endpoint: PushEndpoint?) { - preferences - .edit() - .putString(UNIFIED_PUSH_ENDPOINT_URL, endpoint?.url) - .putString(UNIFIED_PUSH_ENDPOINT_KEY, endpoint?.pubKeySet?.pubKey) - .putString(UNIFIED_PUSH_ENDPOINT_AUTH, endpoint?.pubKeySet?.auth) - .apply() + preferences.edit { + putString(UNIFIED_PUSH_ENDPOINT_URL, endpoint?.url) + putString(UNIFIED_PUSH_ENDPOINT_KEY, endpoint?.pubKeySet?.pubKey) + putString(UNIFIED_PUSH_ENDPOINT_AUTH, endpoint?.pubKeySet?.auth) + } } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt index 162c67afb8..a5e00c501c 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt @@ -14,6 +14,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import at.bitfire.cert4android.CustomCertStore import at.bitfire.davdroid.BuildConfig +import at.bitfire.davdroid.repository.DavCollectionRepository import at.bitfire.davdroid.repository.PreferenceRepository import at.bitfire.davdroid.settings.Settings import at.bitfire.davdroid.settings.SettingsManager @@ -37,7 +38,8 @@ import javax.inject.Inject @HiltViewModel class AppSettingsModel @Inject constructor( @ApplicationContext val context: Context, - private val preference: PreferenceRepository, + private val collectionRepository: DavCollectionRepository, + private val preferences: PreferenceRepository, private val settings: SettingsManager, tasksAppManager: TasksAppManager ) : ViewModel() { @@ -50,9 +52,9 @@ class AppSettingsModel @Inject constructor( .map { powerManager.isIgnoringBatteryOptimizations(BuildConfig.APPLICATION_ID) } .stateIn(viewModelScope, SharingStarted.WhileSubscribed(), false) - fun verboseLogging() = preference.logToFileFlow() + fun verboseLogging() = preferences.logToFileFlow() fun updateVerboseLogging(verbose: Boolean) { - preference.logToFile(verbose) + preferences.logToFile(verbose) } @@ -163,9 +165,17 @@ class AppSettingsModel @Inject constructor( UnifiedPush.removeDistributor(context) UnifiedPush.unregister(context) } else { - // If a distributor was passed, store it and register the app + // If a distributor was passed, store it UnifiedPush.saveDistributor(context, pushDistributor) - UnifiedPush.register(context) + + // … and register it so that UnifiedPushReceiver.onNewEndpoint is called + // TODO: Re-register whenever VAPID key changes/becomes available. Probably this code should be somewhere else. + val vapidKeys = collectionRepository.getVapidKeys() + if (vapidKeys.isNotEmpty()) + for (key in vapidKeys) + UnifiedPush.register(context, vapid = key) + else + UnifiedPush.register(context) } _pushDistributor.value = pushDistributor } From 2adea3469182def4476ace100778f6538bddf1b2 Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Tue, 15 Apr 2025 20:53:52 +0200 Subject: [PATCH 08/20] [WIP] Refactor push registration logic and remove deprecated methods --- .../at/bitfire/davdroid/db/CollectionDao.kt | 10 +- .../at/bitfire/davdroid/db/ServiceDao.kt | 3 + .../davdroid/push/PushRegistrationManager.kt | 149 ++++++++++++++++++ .../davdroid/push/PushRegistrationWorker.kt | 16 +- .../push/PushRegistrationWorkerManager.kt | 9 +- .../davdroid/push/UnifiedPushService.kt | 40 +++-- .../repository/DavCollectionRepository.kt | 4 +- .../repository/DavServiceRepository.kt | 2 + .../repository/PreferenceRepository.kt | 19 --- .../RefreshCollectionsWorker.kt | 8 +- .../bitfire/davdroid/ui/AppSettingsModel.kt | 11 +- 11 files changed, 207 insertions(+), 64 deletions(-) create mode 100644 app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt index 698410a34c..d412b45d0b 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt @@ -35,8 +35,8 @@ interface CollectionDao { @Query("SELECT * FROM collection WHERE pushTopic=:topic AND sync") fun getSyncableByPushTopic(topic: String): Collection? - @Query("SELECT DISTINCT pushVapidKey FROM collection WHERE pushVapidKey IS NOT NULL") - suspend fun getVapidKeys(): List + @Query("SELECT pushVapidKey FROM collection WHERE serviceId=:serviceId AND pushVapidKey IS NOT NULL LIMIT 1") + fun getFirstVapidKey(serviceId: Long): String? @Query("SELECT COUNT(*) FROM collection WHERE serviceId=:serviceId AND type=:type") suspend fun anyOfType(serviceId: Long, @CollectionType type: String): Boolean @@ -75,8 +75,8 @@ interface CollectionDao { * Get a list of collections that are both sync enabled and push capable (supportsWebPush and * pushTopic is available). */ - @Query("SELECT * FROM collection WHERE sync AND supportsWebPush AND pushTopic IS NOT NULL") - suspend fun getPushCapableSyncCollections(): List + @Query("SELECT * FROM collection WHERE serviceId=:serviceId AND sync AND supportsWebPush AND pushTopic IS NOT NULL") + fun getPushCapableSyncCollections(serviceId: Long): List @Query("SELECT * FROM collection WHERE pushSubscription IS NOT NULL AND NOT sync") suspend fun getPushRegisteredAndNotSyncable(): List @@ -119,4 +119,4 @@ interface CollectionDao { @Delete fun delete(collection: Collection) -} +} \ No newline at end of file diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt index 03769448db..fd1639fccd 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt @@ -25,6 +25,9 @@ interface ServiceDao { @Query("SELECT * FROM service WHERE id=:id") fun get(id: Long): Service? + @Query("SELECT * FROM service") + fun getAll(): List + @Insert(onConflict = OnConflictStrategy.REPLACE) fun insertOrReplace(service: Service): Long diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt new file mode 100644 index 0000000000..d33841c333 --- /dev/null +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt @@ -0,0 +1,149 @@ +/* + * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details. + */ + +package at.bitfire.davdroid.push + +import android.content.Context +import at.bitfire.dav4jvm.DavCollection +import at.bitfire.dav4jvm.DavResource +import at.bitfire.dav4jvm.HttpUtils +import at.bitfire.dav4jvm.XmlUtils +import at.bitfire.dav4jvm.XmlUtils.insertTag +import at.bitfire.dav4jvm.property.push.AuthSecret +import at.bitfire.dav4jvm.property.push.PushRegister +import at.bitfire.dav4jvm.property.push.PushResource +import at.bitfire.dav4jvm.property.push.Subscription +import at.bitfire.dav4jvm.property.push.SubscriptionPublicKey +import at.bitfire.dav4jvm.property.push.WebPushSubscription +import at.bitfire.davdroid.db.Collection +import at.bitfire.davdroid.network.HttpClient +import at.bitfire.davdroid.repository.AccountRepository +import at.bitfire.davdroid.repository.DavCollectionRepository +import at.bitfire.davdroid.repository.DavServiceRepository +import dagger.hilt.android.qualifiers.ApplicationContext +import okhttp3.RequestBody.Companion.toRequestBody +import org.unifiedpush.android.connector.UnifiedPush +import org.unifiedpush.android.connector.data.PushEndpoint +import java.io.StringWriter +import java.time.Duration +import java.time.Instant +import java.util.logging.Level +import java.util.logging.Logger +import javax.inject.Inject +import javax.inject.Provider + +class PushRegistrationManager @Inject constructor( + private val accountRepository: AccountRepository, + private val collectionRepository: DavCollectionRepository, + @ApplicationContext private val context: Context, + private val httpClientBuilder: Provider, + private val logger: Logger, + private val serviceRepository: DavServiceRepository +) { + + fun update() { + for (service in serviceRepository.getAll()) + update(service.id) + } + + fun update(serviceId: Long) { + val service = serviceRepository.get(serviceId) ?: return + val vapid = collectionRepository.getVapidKey(serviceId) + + if (vapid != null) + try { + UnifiedPush.register(context, serviceId.toString(), service.accountName, vapid) + } catch (e: UnifiedPush.VapidNotValidException) { + logger.log(Level.WARNING, "Couldn't register invalid VAPID key for service $serviceId", e) + } + else + UnifiedPush.unregister(context, serviceId.toString()) + } + + fun registerSubscription(serviceId: Long, endpoint: PushEndpoint) { + val service = serviceRepository.get(serviceId) ?: return + + val collectionsToRegister = collectionRepository.getPushCapableAndSyncable(serviceId) + if (collectionsToRegister.isEmpty()) + return + + val account = accountRepository.fromName(service.accountName) + httpClientBuilder.get() + .fromAccount(account) + .build() + .use { httpClient -> + for (collection in collectionsToRegister) + try { + val expires = collection.pushSubscriptionExpires + // calculate next run time, but use the duplicate interval for safety (times are not exact) + val nextRun = Instant.now() + Duration.ofDays(2 * PushRegistrationWorkerManager.INTERVAL_DAYS) + if (expires != null && expires >= nextRun.epochSecond) + logger.fine("Push subscription for ${collection.url} is still valid until ${collection.pushSubscriptionExpires}") + else { + // no existing subscription or expiring soon + logger.fine("Registering push subscription for ${collection.url}") + registerSubscription(httpClient, collection, endpoint) + } + } catch (e: Exception) { + logger.log(Level.WARNING, "Couldn't register subscription at CalDAV/CardDAV server", e) + } + } + } + + private fun registerSubscription(httpClient: HttpClient, collection: Collection, endpoint: PushEndpoint) { + // requested expiration time: 3 days + val requestedExpiration = Instant.now() + Duration.ofDays(3) + + val serializer = XmlUtils.newSerializer() + val writer = StringWriter() + serializer.setOutput(writer) + serializer.startDocument("UTF-8", true) + serializer.insertTag(PushRegister.NAME) { + serializer.insertTag(Subscription.NAME) { + // subscription URL + serializer.insertTag(WebPushSubscription.NAME) { + serializer.insertTag(PushResource.NAME) { + text(endpoint.url) + } + endpoint.pubKeySet?.let { pubKeySet -> + serializer.insertTag(SubscriptionPublicKey.NAME) { + attribute(null, "type", "p256dh") + text(pubKeySet.pubKey) + } + serializer.insertTag(AuthSecret.NAME) { + text(pubKeySet.auth) + } + } + } + } + // requested expiration + serializer.insertTag(PushRegister.EXPIRES) { + text(HttpUtils.formatDate(requestedExpiration)) + } + } + serializer.endDocument() + + val xml = writer.toString().toRequestBody(DavResource.MIME_XML) + DavCollection(httpClient.okHttpClient, collection.url).post(xml) { response -> + if (response.isSuccessful) { + // update subscription URL and expiration in DB + val subscriptionUrl = response.header("Location") + val expires = response.header("Expires")?.let { expiresDate -> + HttpUtils.parseDate(expiresDate) + } ?: requestedExpiration + collectionRepository.updatePushSubscription( + id = collection.id, + subscriptionUrl = subscriptionUrl, + expires = expires?.epochSecond + ) + } else + logger.warning("Couldn't register push for ${collection.url}: $response") + } + } + + fun unregisterSubscription(serviceId: Long) { + // TODO + } + +} \ No newline at end of file diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt index 29df297146..4a7835333c 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt @@ -52,27 +52,31 @@ import javax.inject.Provider class PushRegistrationWorker @AssistedInject constructor( @Assisted context: Context, @Assisted workerParameters: WorkerParameters, - private val collectionRepository: DavCollectionRepository, + private val pushRegistrationManager: PushRegistrationManager, + /*private val collectionRepository: DavCollectionRepository, private val httpClientBuilder: Provider, private val logger: Logger, - private val preferenceRepository: PreferenceRepository, + private val preferenceRepository: PreferenceRepository,*/ private val serviceRepository: DavServiceRepository ) : CoroutineWorker(context, workerParameters) { override suspend fun doWork(): Result { - logger.info("Running push registration worker") + /*logger.info("Running push registration worker") try { registerSyncable() unregisterNotSyncable() } catch (_: IOException) { return Result.retry() // retry on I/O errors - } + }*/ + + // update registrations for all services + pushRegistrationManager.update() return Result.success() } - private suspend fun registerPushSubscription(collection: Collection, account: Account, endpoint: PushEndpoint) { + /*private suspend fun registerPushSubscription(collection: Collection, account: Account, endpoint: PushEndpoint) { httpClientBuilder.get() .fromAccount(account) .build() @@ -198,6 +202,6 @@ class PushRegistrationWorker @AssistedInject constructor( } } } - } + }*/ } \ No newline at end of file diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorkerManager.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorkerManager.kt index 453f32a52c..83885aa233 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorkerManager.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorkerManager.kt @@ -12,12 +12,7 @@ import androidx.work.NetworkType import androidx.work.PeriodicWorkRequest import androidx.work.WorkManager import at.bitfire.davdroid.repository.DavCollectionRepository -import dagger.Binds -import dagger.Module -import dagger.hilt.InstallIn import dagger.hilt.android.qualifiers.ApplicationContext -import dagger.hilt.components.SingletonComponent -import dagger.multibindings.IntoSet import kotlinx.coroutines.runBlocking import java.util.concurrent.TimeUnit import java.util.logging.Logger @@ -70,7 +65,7 @@ class PushRegistrationWorkerManager @Inject constructor( const val INTERVAL_DAYS = 1L } - + /* /** * Listener that enqueues a push registration worker when the collection list changes. */ @@ -95,4 +90,6 @@ class PushRegistrationWorkerManager @Inject constructor( @IntoSet fun listener(impl: CollectionsListener): DavCollectionRepository.OnChangeListener } + */ + } \ No newline at end of file diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt index 34e37cd467..cc55dda7c9 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt @@ -8,7 +8,6 @@ import at.bitfire.davdroid.db.Collection.Companion.TYPE_ADDRESSBOOK 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.sync.SyncDataType import at.bitfire.davdroid.sync.TasksAppManager import at.bitfire.davdroid.sync.worker.SyncWorkerManager @@ -40,14 +39,11 @@ class UnifiedPushService: PushService() { @Inject lateinit var serviceRepository: DavServiceRepository - @Inject - lateinit var preferenceRepository: PreferenceRepository - @Inject lateinit var parsePushMessage: PushMessageParser @Inject - lateinit var pushRegistrationWorkerManager: PushRegistrationWorkerManager + lateinit var pushRegistrationManager: PushRegistrationManager @Inject lateinit var tasksAppManager: Lazy @@ -55,31 +51,41 @@ class UnifiedPushService: PushService() { @Inject lateinit var syncWorkerManager: SyncWorkerManager + val dispatcher = Dispatchers.IO.limitedParallelism(1) + val scope = CoroutineScope(dispatcher) - override fun onNewEndpoint(endpoint: PushEndpoint, instance: String) { - logger.info("Got UnifiedPush endpoint: ${endpoint.url}") - // remember new endpoint - preferenceRepository.unifiedPushEndpoint(endpoint) + override fun onNewEndpoint(endpoint: PushEndpoint, instance: String) { + val serviceId = instance.toLongOrNull() ?: return + logger.log(Level.FINE, "Got UnifiedPush endpoint for service $serviceId", endpoint.url) // register new endpoint at CalDAV/CardDAV servers - pushRegistrationWorkerManager.updatePeriodicWorker() + scope.launch { + pushRegistrationManager.registerSubscription(serviceId, endpoint) + } } override fun onRegistrationFailed(reason: FailedReason, instance: String) { - logger.warning("UnifiedPush registration failed: $reason") - // reset known endpoint to make sure nothing is stored when not registered - preferenceRepository.unifiedPushEndpoint(null) + val serviceId = instance.toLongOrNull() ?: return + logger.warning("UnifiedPush registration failed for service $serviceId: $reason") + + // unregister subscriptions + scope.launch { + pushRegistrationManager.unregisterSubscription(serviceId) + } } override fun onUnregistered(instance: String) { - logger.warning("UnifiedPush unregistered") - // reset known endpoint - preferenceRepository.unifiedPushEndpoint(null) + val serviceId = instance.toLongOrNull() ?: return + logger.warning("UnifiedPush unregistered for service $serviceId") + + scope.launch { + pushRegistrationManager.unregisterSubscription(serviceId) + } } override fun onMessage(message: PushMessage, instance: String) { - CoroutineScope(Dispatchers.Default).launch { + scope.launch { if (!message.decrypted) { logger.severe("Received a push message that could not be decrypted.") return@launch diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt index fe9834a47a..8232dc0afe 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt @@ -212,11 +212,11 @@ class DavCollectionRepository @Inject constructor( fun getSyncTaskLists(serviceId: Long) = dao.getSyncTaskLists(serviceId) /** Returns all collections that are both selected for synchronization and push-capable. */ - suspend fun getPushCapableAndSyncable() = dao.getPushCapableSyncCollections() + fun getPushCapableAndSyncable(serviceId: Long) = dao.getPushCapableSyncCollections(serviceId) suspend fun getPushRegisteredAndNotSyncable() = dao.getPushRegisteredAndNotSyncable() - suspend fun getVapidKeys() = dao.getVapidKeys() + fun getVapidKey(serviceId: Long) = dao.getFirstVapidKey(serviceId) /** * Inserts or updates the collection. diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt index bdfb8c41bf..0e2fd36b41 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt @@ -20,6 +20,8 @@ class DavServiceRepository @Inject constructor( fun get(id: Long): Service? = dao.get(id) + fun getAll(): List = dao.getAll() + fun getByAccountAndType(name: String, @ServiceType serviceType: String): Service? = dao.getByAccountAndType(name, serviceType) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/PreferenceRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/PreferenceRepository.kt index e3bc718e30..c9c01405f3 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/repository/PreferenceRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/repository/PreferenceRepository.kt @@ -12,8 +12,6 @@ import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.callbackFlow -import org.unifiedpush.android.connector.data.PublicKeySet -import org.unifiedpush.android.connector.data.PushEndpoint import javax.inject.Inject /** @@ -58,23 +56,6 @@ class PreferenceRepository @Inject constructor( } - fun unifiedPushEndpoint(): PushEndpoint? { - val url = preferences.getString(UNIFIED_PUSH_ENDPOINT_URL, null) ?: return null - val key = preferences.getString(UNIFIED_PUSH_ENDPOINT_KEY, null) - val auth = preferences.getString(UNIFIED_PUSH_ENDPOINT_AUTH, null) - val publicKeySet = if (key != null && auth != null) PublicKeySet(key, auth) else null - return PushEndpoint(url, publicKeySet) - } - - fun unifiedPushEndpoint(endpoint: PushEndpoint?) { - preferences.edit { - putString(UNIFIED_PUSH_ENDPOINT_URL, endpoint?.url) - putString(UNIFIED_PUSH_ENDPOINT_KEY, endpoint?.pubKeySet?.pubKey) - putString(UNIFIED_PUSH_ENDPOINT_AUTH, endpoint?.pubKeySet?.auth) - } - } - - // helpers private fun observeAsFlow(keyToObserve: String, getValue: () -> T): Flow = diff --git a/app/src/main/kotlin/at/bitfire/davdroid/servicedetection/RefreshCollectionsWorker.kt b/app/src/main/kotlin/at/bitfire/davdroid/servicedetection/RefreshCollectionsWorker.kt index 4f66306a38..d3686f4bc6 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/servicedetection/RefreshCollectionsWorker.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/servicedetection/RefreshCollectionsWorker.kt @@ -23,10 +23,12 @@ import androidx.work.WorkInfo import androidx.work.WorkManager import androidx.work.WorkerParameters import at.bitfire.dav4jvm.exception.UnauthorizedException -import at.bitfire.davdroid.sync.account.InvalidAccountException import at.bitfire.davdroid.R import at.bitfire.davdroid.network.HttpClient +import at.bitfire.davdroid.push.PushRegistrationManager import at.bitfire.davdroid.repository.DavServiceRepository +import at.bitfire.davdroid.servicedetection.RefreshCollectionsWorker.Companion.ARG_SERVICE_ID +import at.bitfire.davdroid.sync.account.InvalidAccountException import at.bitfire.davdroid.ui.DebugInfoActivity import at.bitfire.davdroid.ui.NotificationRegistry import at.bitfire.davdroid.ui.account.AccountSettingsActivity @@ -64,6 +66,7 @@ class RefreshCollectionsWorker @AssistedInject constructor( private val httpClientBuilder: HttpClient.Builder, private val logger: Logger, private val notificationRegistry: NotificationRegistry, + private val pushRegistrationManager: PushRegistrationManager, serviceRepository: DavServiceRepository ): CoroutineWorker(appContext, workerParams) { @@ -199,6 +202,9 @@ class RefreshCollectionsWorker @AssistedInject constructor( return Result.failure() } + // update push registrations + pushRegistrationManager.update(serviceId) + // Success return Result.success() } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt index a5e00c501c..a2f6c23466 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt @@ -14,6 +14,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import at.bitfire.cert4android.CustomCertStore import at.bitfire.davdroid.BuildConfig +import at.bitfire.davdroid.push.PushRegistrationManager import at.bitfire.davdroid.repository.DavCollectionRepository import at.bitfire.davdroid.repository.PreferenceRepository import at.bitfire.davdroid.settings.Settings @@ -40,6 +41,7 @@ class AppSettingsModel @Inject constructor( @ApplicationContext val context: Context, private val collectionRepository: DavCollectionRepository, private val preferences: PreferenceRepository, + private val pushRegistrationManager: PushRegistrationManager, private val settings: SettingsManager, tasksAppManager: TasksAppManager ) : ViewModel() { @@ -163,19 +165,12 @@ class AppSettingsModel @Inject constructor( if (pushDistributor == null) { // Disable UnifiedPush if the distributor given is null UnifiedPush.removeDistributor(context) - UnifiedPush.unregister(context) } else { // If a distributor was passed, store it UnifiedPush.saveDistributor(context, pushDistributor) // … and register it so that UnifiedPushReceiver.onNewEndpoint is called - // TODO: Re-register whenever VAPID key changes/becomes available. Probably this code should be somewhere else. - val vapidKeys = collectionRepository.getVapidKeys() - if (vapidKeys.isNotEmpty()) - for (key in vapidKeys) - UnifiedPush.register(context, vapid = key) - else - UnifiedPush.register(context) + pushRegistrationManager.update() } _pushDistributor.value = pushDistributor } From 1dadfdad3a450bad003f539f4ddc8d8e8acd2c9a Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Wed, 16 Apr 2025 15:41:14 +0200 Subject: [PATCH 09/20] [WIP] Remove PushRegistrationWorkerManager and refactor PushRegistrationManager --- .../kotlin/at/bitfire/davdroid/TestModules.kt | 14 -- .../at/bitfire/davdroid/db/CollectionDao.kt | 11 +- .../davdroid/push/PushRegistrationManager.kt | 221 +++++++++++++++--- .../davdroid/push/PushRegistrationWorker.kt | 177 +------------- .../push/PushRegistrationWorkerManager.kt | 95 -------- .../davdroid/push/UnifiedPushService.kt | 27 +-- .../repository/DavCollectionRepository.kt | 7 +- .../migration/AccountSettingsMigration18.kt | 28 ++- 8 files changed, 235 insertions(+), 345 deletions(-) delete mode 100644 app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorkerManager.kt diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/TestModules.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/TestModules.kt index 5f96cc89f2..c1e426fe6a 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/TestModules.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/TestModules.kt @@ -4,8 +4,6 @@ package at.bitfire.davdroid -import at.bitfire.davdroid.push.PushRegistrationWorkerManager -import at.bitfire.davdroid.repository.DavCollectionRepository import at.bitfire.davdroid.startup.StartupPlugin import at.bitfire.davdroid.startup.TasksAppWatcher import dagger.Module @@ -13,18 +11,6 @@ import dagger.hilt.components.SingletonComponent import dagger.hilt.testing.TestInstallIn import dagger.multibindings.Multibinds -// remove PushRegistrationWorkerModule from Android tests -@Module -@TestInstallIn( - components = [SingletonComponent::class], - replaces = [PushRegistrationWorkerManager.PushRegistrationWorkerModule::class] -) -abstract class TestPushRegistrationWorkerModule { - // provides empty set of listeners - @Multibinds - abstract fun empty(): Set -} - // remove TasksAppWatcherModule from Android tests @Module @TestInstallIn( diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt index d412b45d0b..cb859e4f6b 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt @@ -24,7 +24,7 @@ interface CollectionDao { fun getFlow(id: Long): Flow @Query("SELECT * FROM collection WHERE serviceId=:serviceId") - fun getByService(serviceId: Long): List + suspend fun getByService(serviceId: Long): List @Query("SELECT * FROM collection WHERE serviceId=:serviceId AND homeSetId IS :homeSetId") fun getByServiceAndHomeset(serviceId: Long, homeSetId: Long?): List @@ -36,7 +36,7 @@ interface CollectionDao { fun getSyncableByPushTopic(topic: String): Collection? @Query("SELECT pushVapidKey FROM collection WHERE serviceId=:serviceId AND pushVapidKey IS NOT NULL LIMIT 1") - fun getFirstVapidKey(serviceId: Long): String? + suspend fun getFirstVapidKey(serviceId: Long): String? @Query("SELECT COUNT(*) FROM collection WHERE serviceId=:serviceId AND type=:type") suspend fun anyOfType(serviceId: Long, @CollectionType type: String): Boolean @@ -78,8 +78,11 @@ interface CollectionDao { @Query("SELECT * FROM collection WHERE serviceId=:serviceId AND sync AND supportsWebPush AND pushTopic IS NOT NULL") fun getPushCapableSyncCollections(serviceId: Long): List - @Query("SELECT * FROM collection WHERE pushSubscription IS NOT NULL AND NOT sync") - suspend fun getPushRegisteredAndNotSyncable(): List + @Query("SELECT * FROM collection WHERE serviceId=:serviceId AND pushSubscription IS NOT NULL") + suspend fun getPushRegistered(serviceId: Long): List + + @Query("SELECT * FROM collection WHERE serviceId=:serviceId AND pushSubscription IS NOT NULL AND NOT sync") + suspend fun getPushRegisteredAndNotSyncable(serviceId: Long): List @Insert(onConflict = OnConflictStrategy.IGNORE) fun insert(collection: Collection): Long diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt index d33841c333..e11319a561 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt @@ -5,11 +5,18 @@ package at.bitfire.davdroid.push import android.content.Context +import androidx.work.BackoffPolicy +import androidx.work.Constraints +import androidx.work.ExistingPeriodicWorkPolicy +import androidx.work.NetworkType +import androidx.work.PeriodicWorkRequest +import androidx.work.WorkManager import at.bitfire.dav4jvm.DavCollection import at.bitfire.dav4jvm.DavResource import at.bitfire.dav4jvm.HttpUtils import at.bitfire.dav4jvm.XmlUtils import at.bitfire.dav4jvm.XmlUtils.insertTag +import at.bitfire.dav4jvm.exception.DavException import at.bitfire.dav4jvm.property.push.AuthSecret import at.bitfire.dav4jvm.property.push.PushRegister import at.bitfire.dav4jvm.property.push.PushResource @@ -17,24 +24,33 @@ import at.bitfire.dav4jvm.property.push.Subscription import at.bitfire.dav4jvm.property.push.SubscriptionPublicKey import at.bitfire.dav4jvm.property.push.WebPushSubscription import at.bitfire.davdroid.db.Collection +import at.bitfire.davdroid.db.Service import at.bitfire.davdroid.network.HttpClient import at.bitfire.davdroid.repository.AccountRepository import at.bitfire.davdroid.repository.DavCollectionRepository import at.bitfire.davdroid.repository.DavServiceRepository +import dagger.Lazy import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.runInterruptible +import kotlinx.coroutines.withContext +import okhttp3.HttpUrl +import okhttp3.HttpUrl.Companion.toHttpUrlOrNull import okhttp3.RequestBody.Companion.toRequestBody import org.unifiedpush.android.connector.UnifiedPush import org.unifiedpush.android.connector.data.PushEndpoint import java.io.StringWriter import java.time.Duration import java.time.Instant +import java.util.concurrent.TimeUnit import java.util.logging.Level import java.util.logging.Logger import javax.inject.Inject import javax.inject.Provider class PushRegistrationManager @Inject constructor( - private val accountRepository: AccountRepository, + private val accountRepository: Lazy, private val collectionRepository: DavCollectionRepository, @ApplicationContext private val context: Context, private val httpClientBuilder: Provider, @@ -42,13 +58,29 @@ class PushRegistrationManager @Inject constructor( private val serviceRepository: DavServiceRepository ) { - fun update() { + /** + * Updates all push registrations and subscriptions so that if Push is available, it's up-to-date and + * working for all database services. + * + * Also makes sure that the [PushRegistrationWorker] is enabled if there's a Push-enabled collection. + */ + suspend fun update() = withContext(dispatcher) { for (service in serviceRepository.getAll()) - update(service.id) + updateService(service.id) + + updatePeriodicWorker() } - fun update(serviceId: Long) { - val service = serviceRepository.get(serviceId) ?: return + /** + * Same as [update], but for a specific database service. + */ + suspend fun update(serviceId: Long) { + updateService(serviceId) + updatePeriodicWorker() + } + + private suspend fun updateService(serviceId: Long) = withContext(dispatcher) { + val service = serviceRepository.get(serviceId) ?: return@withContext val vapid = collectionRepository.getVapidKey(serviceId) if (vapid != null) @@ -59,31 +91,45 @@ class PushRegistrationManager @Inject constructor( } else UnifiedPush.unregister(context, serviceId.toString()) + + // UnifiedPush has now been called. It will do its work and then call back to UnifiedPushService, which + // will then call processSubscription or removeSubscription. } - fun registerSubscription(serviceId: Long, endpoint: PushEndpoint) { - val service = serviceRepository.get(serviceId) ?: return - val collectionsToRegister = collectionRepository.getPushCapableAndSyncable(serviceId) - if (collectionsToRegister.isEmpty()) + /** + * Called when a subscription (endpoint) is available for the given service. + * + * Uses the subscription to subscribe to syncable collections, and then unsubscribes from non-syncable collections. + */ + internal suspend fun processSubscription(serviceId: Long, endpoint: PushEndpoint) = withContext(dispatcher) { + val service = serviceRepository.get(serviceId) ?: return@withContext + + subscribeSyncable(service, endpoint) + unsubscribeNotSyncable(service) + } + + private suspend fun subscribeSyncable(service: Service, endpoint: PushEndpoint) { + val subscribeTo = collectionRepository.getPushCapableAndSyncable(service.id) + if (subscribeTo.isEmpty()) return - val account = accountRepository.fromName(service.accountName) + val account = accountRepository.get().fromName(service.accountName) httpClientBuilder.get() .fromAccount(account) .build() .use { httpClient -> - for (collection in collectionsToRegister) + for (collection in subscribeTo) try { val expires = collection.pushSubscriptionExpires // calculate next run time, but use the duplicate interval for safety (times are not exact) - val nextRun = Instant.now() + Duration.ofDays(2 * PushRegistrationWorkerManager.INTERVAL_DAYS) + val nextRun = Instant.now() + Duration.ofDays(2 * WORKER_INTERVAL_DAYS) if (expires != null && expires >= nextRun.epochSecond) logger.fine("Push subscription for ${collection.url} is still valid until ${collection.pushSubscriptionExpires}") else { // no existing subscription or expiring soon logger.fine("Registering push subscription for ${collection.url}") - registerSubscription(httpClient, collection, endpoint) + subscribe(httpClient, collection, endpoint) } } catch (e: Exception) { logger.log(Level.WARNING, "Couldn't register subscription at CalDAV/CardDAV server", e) @@ -91,7 +137,57 @@ class PushRegistrationManager @Inject constructor( } } - private fun registerSubscription(httpClient: HttpClient, collection: Collection, endpoint: PushEndpoint) { + private suspend fun unsubscribeNotSyncable(service: Service) { + val unsubscribeFrom = collectionRepository.getPushRegisteredAndNotSyncable(service.id) + if (unsubscribeFrom.isEmpty()) + return + + val account = accountRepository.get().fromName(service.accountName) + httpClientBuilder.get() + .fromAccount(account) + .build() + .use { httpClient -> + for (collection in unsubscribeFrom) + collection.pushSubscription?.toHttpUrlOrNull()?.let { url -> + logger.info("Unregistering push for ${collection.url}") + unsubscribe(httpClient, collection, url) + } + } + } + + /** + * Called when no subscription is available (anymore) for the given service. + * + * Unsubscribes from all collections. + */ + internal suspend fun removeSubscription(serviceId: Long) { + val service = serviceRepository.get(serviceId) ?: return + val unsubscribeFrom = collectionRepository.getPushRegistered(service.id) + if (unsubscribeFrom.isEmpty()) + return + + val account = accountRepository.get().fromName(service.accountName) + httpClientBuilder.get() + .fromAccount(account) + .build() + .use { httpClient -> + for (collection in unsubscribeFrom) + collection.pushSubscription?.toHttpUrlOrNull()?.let { url -> + logger.info("Unregistering push for ${collection.url}") + unsubscribe(httpClient, collection, url) + } + } + } + + + /** + * Registers the subscription to a given collection ("subscribe to a collection"). + * + * @param httpClient HTTP client to use + * @param collection collection to subscribe to + * @param endpoint subscription to register + */ + private suspend fun subscribe(httpClient: HttpClient, collection: Collection, endpoint: PushEndpoint) { // requested expiration time: 3 days val requestedExpiration = Instant.now() + Duration.ofDays(3) @@ -124,26 +220,89 @@ class PushRegistrationManager @Inject constructor( } serializer.endDocument() - val xml = writer.toString().toRequestBody(DavResource.MIME_XML) - DavCollection(httpClient.okHttpClient, collection.url).post(xml) { response -> - if (response.isSuccessful) { - // update subscription URL and expiration in DB - val subscriptionUrl = response.header("Location") - val expires = response.header("Expires")?.let { expiresDate -> - HttpUtils.parseDate(expiresDate) - } ?: requestedExpiration - collectionRepository.updatePushSubscription( - id = collection.id, - subscriptionUrl = subscriptionUrl, - expires = expires?.epochSecond - ) - } else - logger.warning("Couldn't register push for ${collection.url}: $response") + runInterruptible { + val xml = writer.toString().toRequestBody(DavResource.MIME_XML) + DavCollection(httpClient.okHttpClient, collection.url).post(xml) { response -> + if (response.isSuccessful) { + // update subscription URL and expiration in DB + val subscriptionUrl = response.header("Location") + val expires = response.header("Expires")?.let { expiresDate -> + HttpUtils.parseDate(expiresDate) + } ?: requestedExpiration + collectionRepository.updatePushSubscription( + id = collection.id, + subscriptionUrl = subscriptionUrl, + expires = expires?.epochSecond + ) + } else + logger.warning("Couldn't register push for ${collection.url}: $response") + } + } + } + + private suspend fun unsubscribe(httpClient: HttpClient, collection: Collection, url: HttpUrl) { + runInterruptible { + try { + DavResource(httpClient.okHttpClient, url).delete { + // deleted + } + } catch (e: DavException) { + logger.log(Level.WARNING, "Couldn't unregister push for ${collection.url}", e) + } + + // remove registration URL from DB in any case + collectionRepository.updatePushSubscription( + id = collection.id, + subscriptionUrl = null, + expires = null + ) + } + } + + + /** + * Determines whether there are any push-capable collections and updates the periodic worker accordingly. + * + * If there are push-capable collections, a unique periodic worker with an initial delay of 5 seconds is enqueued. + * A potentially existing worker is replaced, so that the first run should be soon. + * + * Otherwise, a potentially existing worker is cancelled. + */ + fun updatePeriodicWorker() { + val workerNeeded = runBlocking { + collectionRepository.anyPushCapable() + } + + val workManager = WorkManager.getInstance(context) + if (workerNeeded) { + logger.info("Enqueuing periodic PushRegistrationWorker") + workManager.enqueueUniquePeriodicWork( + WORKER_UNIQUE_NAME, + ExistingPeriodicWorkPolicy.UPDATE, + PeriodicWorkRequest.Builder(PushRegistrationWorker::class, WORKER_INTERVAL_DAYS, TimeUnit.DAYS) + .setInitialDelay(5, TimeUnit.SECONDS) + .setConstraints( + Constraints.Builder() + .setRequiredNetworkType(NetworkType.CONNECTED) + .build() + ) + .setBackoffCriteria(BackoffPolicy.EXPONENTIAL, 1, TimeUnit.MINUTES) + .build() + ) + } else { + logger.info("Cancelling periodic PushRegistrationWorker") + workManager.cancelUniqueWork(WORKER_UNIQUE_NAME) } } - fun unregisterSubscription(serviceId: Long) { - // TODO + + companion object { + + private const val WORKER_UNIQUE_NAME = "push-registration" + const val WORKER_INTERVAL_DAYS = 1L + + val dispatcher = Dispatchers.IO.limitedParallelism(1) + } } \ No newline at end of file diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt index 4a7835333c..300aac586e 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt @@ -4,71 +4,32 @@ package at.bitfire.davdroid.push -import android.accounts.Account import android.content.Context import androidx.hilt.work.HiltWorker import androidx.work.CoroutineWorker import androidx.work.WorkerParameters -import at.bitfire.dav4jvm.DavCollection -import at.bitfire.dav4jvm.DavResource -import at.bitfire.dav4jvm.HttpUtils -import at.bitfire.dav4jvm.XmlUtils -import at.bitfire.dav4jvm.XmlUtils.insertTag -import at.bitfire.dav4jvm.exception.DavException -import at.bitfire.dav4jvm.property.push.AuthSecret -import at.bitfire.dav4jvm.property.push.PushRegister -import at.bitfire.dav4jvm.property.push.PushResource -import at.bitfire.dav4jvm.property.push.Subscription -import at.bitfire.dav4jvm.property.push.SubscriptionPublicKey -import at.bitfire.dav4jvm.property.push.WebPushSubscription -import at.bitfire.davdroid.R -import at.bitfire.davdroid.db.Collection -import at.bitfire.davdroid.network.HttpClient -import at.bitfire.davdroid.repository.DavCollectionRepository import at.bitfire.davdroid.repository.DavServiceRepository -import at.bitfire.davdroid.repository.PreferenceRepository import dagger.assisted.Assisted import dagger.assisted.AssistedInject -import kotlinx.coroutines.runInterruptible -import okhttp3.HttpUrl -import okhttp3.HttpUrl.Companion.toHttpUrlOrNull -import okhttp3.RequestBody.Companion.toRequestBody -import org.unifiedpush.android.connector.data.PushEndpoint -import java.io.IOException -import java.io.StringWriter -import java.time.Duration -import java.time.Instant -import java.util.logging.Level import java.util.logging.Logger -import javax.inject.Provider /** - * Worker that registers push for all collections that support it. - * To be run as soon as a collection that supports push is changed (selected for sync status - * changes, or collection is created, deleted, etc). + * Worker that runs regularly and initiates push registration updates for all collections. + * + * Managed by [PushRegistrationManager]. */ @Suppress("unused") @HiltWorker class PushRegistrationWorker @AssistedInject constructor( @Assisted context: Context, @Assisted workerParameters: WorkerParameters, - private val pushRegistrationManager: PushRegistrationManager, - /*private val collectionRepository: DavCollectionRepository, - private val httpClientBuilder: Provider, private val logger: Logger, - private val preferenceRepository: PreferenceRepository,*/ + private val pushRegistrationManager: PushRegistrationManager, private val serviceRepository: DavServiceRepository ) : CoroutineWorker(context, workerParameters) { override suspend fun doWork(): Result { - /*logger.info("Running push registration worker") - - try { - registerSyncable() - unregisterNotSyncable() - } catch (_: IOException) { - return Result.retry() // retry on I/O errors - }*/ + logger.info("Running push registration worker") // update registrations for all services pushRegistrationManager.update() @@ -76,132 +37,4 @@ class PushRegistrationWorker @AssistedInject constructor( return Result.success() } - /*private suspend fun registerPushSubscription(collection: Collection, account: Account, endpoint: PushEndpoint) { - httpClientBuilder.get() - .fromAccount(account) - .build() - .use { client -> - runInterruptible { - val httpClient = client.okHttpClient - - // requested expiration time: 3 days - val requestedExpiration = Instant.now() + Duration.ofDays(3) - - val serializer = XmlUtils.newSerializer() - val writer = StringWriter() - serializer.setOutput(writer) - serializer.startDocument("UTF-8", true) - serializer.insertTag(PushRegister.NAME) { - serializer.insertTag(Subscription.NAME) { - // subscription URL - serializer.insertTag(WebPushSubscription.NAME) { - serializer.insertTag(PushResource.NAME) { - text(endpoint.url) - } - endpoint.pubKeySet?.let { pubKeySet -> - serializer.insertTag(SubscriptionPublicKey.NAME) { - attribute(null, "type", "p256dh") - text(pubKeySet.pubKey) - } - serializer.insertTag(AuthSecret.NAME) { - text(pubKeySet.auth) - } - } - } - } - // requested expiration - serializer.insertTag(PushRegister.EXPIRES) { - text(HttpUtils.formatDate(requestedExpiration)) - } - } - serializer.endDocument() - - val xml = writer.toString().toRequestBody(DavResource.MIME_XML) - DavCollection(httpClient, collection.url).post(xml) { response -> - if (response.isSuccessful) { - val subscriptionUrl = response.header("Location") - val expires = response.header("Expires")?.let { expiresDate -> - HttpUtils.parseDate(expiresDate) - } ?: requestedExpiration - collectionRepository.updatePushSubscription( - id = collection.id, - subscriptionUrl = subscriptionUrl, - expires = expires?.epochSecond - ) - } else - logger.warning("Couldn't register push for ${collection.url}: $response") - } - } - } - } - - private suspend fun registerSyncable() { - val endpoint = preferenceRepository.unifiedPushEndpoint() - - // register push subscription for syncable collections - if (endpoint != null) { - // subscribe to collections - for (collection in collectionRepository.getPushCapableAndSyncable()) { - val expires = collection.pushSubscriptionExpires - // calculate next run time, but use the duplicate interval for safety (times are not exact) - val nextRun = Instant.now() + Duration.ofDays(2 * PushRegistrationWorkerManager.INTERVAL_DAYS) - if (expires != null && expires >= nextRun.epochSecond) { - logger.fine("Push subscription for ${collection.url} is still valid until ${collection.pushSubscriptionExpires}") - continue - } - - // no existing subscription or expiring soon - logger.info("Registering push for ${collection.url}") - serviceRepository.get(collection.serviceId)?.let { service -> - val account = Account(service.accountName, applicationContext.getString(R.string.account_type)) - try { - registerPushSubscription(collection, account, endpoint) - } catch (e: DavException) { - // catch possible per-collection exception so that all collections can be processed - logger.log(Level.WARNING, "Couldn't register push for ${collection.url}", e) - } - } - } - } else - logger.info("No UnifiedPush endpoint configured") - } - - private suspend fun unregisterPushSubscription(collection: Collection, account: Account, url: HttpUrl) { - httpClientBuilder.get() - .fromAccount(account) - .build() - .use { httpClient -> - runInterruptible { - val httpClient = httpClient.okHttpClient - - try { - DavResource(httpClient, url).delete { - // deleted - } - } catch (e: DavException) { - logger.log(Level.WARNING, "Couldn't unregister push for ${collection.url}", e) - } - - // remove registration URL from DB in any case - collectionRepository.updatePushSubscription( - id = collection.id, - subscriptionUrl = null, - expires = null - ) - } - } - } - - private suspend fun unregisterNotSyncable() { - for (collection in collectionRepository.getPushRegisteredAndNotSyncable()) { - logger.info("Unregistering push for ${collection.url}") - collection.pushSubscription?.toHttpUrlOrNull()?.let { url -> - serviceRepository.get(collection.serviceId)?.let { service -> - val account = Account(service.accountName, applicationContext.getString(R.string.account_type)) - unregisterPushSubscription(collection, account, url) - } - } - } - }*/ - } \ No newline at end of file diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorkerManager.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorkerManager.kt deleted file mode 100644 index 83885aa233..0000000000 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorkerManager.kt +++ /dev/null @@ -1,95 +0,0 @@ -/* - * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details. - */ - -package at.bitfire.davdroid.push - -import android.content.Context -import androidx.work.BackoffPolicy -import androidx.work.Constraints -import androidx.work.ExistingPeriodicWorkPolicy -import androidx.work.NetworkType -import androidx.work.PeriodicWorkRequest -import androidx.work.WorkManager -import at.bitfire.davdroid.repository.DavCollectionRepository -import dagger.hilt.android.qualifiers.ApplicationContext -import kotlinx.coroutines.runBlocking -import java.util.concurrent.TimeUnit -import java.util.logging.Logger -import javax.inject.Inject - -class PushRegistrationWorkerManager @Inject constructor( - @ApplicationContext val context: Context, - val collectionRepository: DavCollectionRepository, - val logger: Logger -) { - - /** - * Determines whether there are any push-capable collections and updates the periodic worker accordingly. - * - * If there are push-capable collections, a unique periodic worker with an initial delay of 5 seconds is enqueued. - * A potentially existing worker is replaced, so that the first run should be soon. - * - * Otherwise, a potentially existing worker is cancelled. - */ - fun updatePeriodicWorker() { - val workerNeeded = runBlocking { - collectionRepository.anyPushCapable() - } - - val workManager = WorkManager.getInstance(context) - if (workerNeeded) { - logger.info("Enqueuing periodic PushRegistrationWorker") - workManager.enqueueUniquePeriodicWork( - UNIQUE_WORK_NAME, - ExistingPeriodicWorkPolicy.CANCEL_AND_REENQUEUE, - PeriodicWorkRequest.Builder(PushRegistrationWorker::class, INTERVAL_DAYS, TimeUnit.DAYS) - .setInitialDelay(5, TimeUnit.SECONDS) - .setConstraints( - Constraints.Builder() - .setRequiredNetworkType(NetworkType.CONNECTED) - .build() - ) - .setBackoffCriteria(BackoffPolicy.EXPONENTIAL, 1, TimeUnit.MINUTES) - .build() - ) - } else { - logger.info("Cancelling periodic PushRegistrationWorker") - workManager.cancelUniqueWork(UNIQUE_WORK_NAME) - } - } - - - companion object { - private const val UNIQUE_WORK_NAME = "push-registration" - const val INTERVAL_DAYS = 1L - } - - /* - /** - * Listener that enqueues a push registration worker when the collection list changes. - */ - class CollectionsListener @Inject constructor( - @ApplicationContext val context: Context, - val workerManager: PushRegistrationWorkerManager - ): DavCollectionRepository.OnChangeListener { - - override fun onCollectionsChanged() { - workerManager.updatePeriodicWorker() - } - - } - - /** - * Hilt module that registers [CollectionsListener] in [DavCollectionRepository]. - */ - @Module - @InstallIn(SingletonComponent::class) - interface PushRegistrationWorkerModule { - @Binds - @IntoSet - fun listener(impl: CollectionsListener): DavCollectionRepository.OnChangeListener - } - */ - -} \ No newline at end of file diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt index cc55dda7c9..3bebe6b800 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt @@ -13,9 +13,7 @@ import at.bitfire.davdroid.sync.TasksAppManager import at.bitfire.davdroid.sync.worker.SyncWorkerManager import dagger.Lazy import dagger.hilt.android.AndroidEntryPoint -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking import org.unifiedpush.android.connector.FailedReason import org.unifiedpush.android.connector.PushService import org.unifiedpush.android.connector.data.PushEndpoint @@ -25,14 +23,14 @@ import java.util.logging.Logger import javax.inject.Inject @AndroidEntryPoint -class UnifiedPushService: PushService() { +class UnifiedPushService : PushService() { @Inject lateinit var accountRepository: AccountRepository @Inject lateinit var collectionRepository: DavCollectionRepository - + @Inject lateinit var logger: Logger @@ -51,17 +49,14 @@ class UnifiedPushService: PushService() { @Inject lateinit var syncWorkerManager: SyncWorkerManager - val dispatcher = Dispatchers.IO.limitedParallelism(1) - val scope = CoroutineScope(dispatcher) - override fun onNewEndpoint(endpoint: PushEndpoint, instance: String) { val serviceId = instance.toLongOrNull() ?: return logger.log(Level.FINE, "Got UnifiedPush endpoint for service $serviceId", endpoint.url) // register new endpoint at CalDAV/CardDAV servers - scope.launch { - pushRegistrationManager.registerSubscription(serviceId, endpoint) + runBlocking { + pushRegistrationManager.processSubscription(serviceId, endpoint) } } @@ -70,8 +65,8 @@ class UnifiedPushService: PushService() { logger.warning("UnifiedPush registration failed for service $serviceId: $reason") // unregister subscriptions - scope.launch { - pushRegistrationManager.unregisterSubscription(serviceId) + runBlocking { + pushRegistrationManager.removeSubscription(serviceId) } } @@ -79,16 +74,16 @@ class UnifiedPushService: PushService() { val serviceId = instance.toLongOrNull() ?: return logger.warning("UnifiedPush unregistered for service $serviceId") - scope.launch { - pushRegistrationManager.unregisterSubscription(serviceId) + runBlocking { + pushRegistrationManager.removeSubscription(serviceId) } } override fun onMessage(message: PushMessage, instance: String) { - scope.launch { + runBlocking { if (!message.decrypted) { logger.severe("Received a push message that could not be decrypted.") - return@launch + return@runBlocking } val messageXml = message.content.toString(Charsets.UTF_8) logger.log(Level.INFO, "Received push message", messageXml) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt index 8232dc0afe..61bf62dc8e 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt @@ -199,7 +199,7 @@ class DavCollectionRepository @Inject constructor( fun getFlow(id: Long) = dao.getFlow(id) - fun getByService(serviceId: Long) = dao.getByService(serviceId) + suspend fun getByService(serviceId: Long) = dao.getByService(serviceId) fun getByServiceAndUrl(serviceId: Long, url: String) = dao.getByServiceAndUrl(serviceId, url) @@ -214,9 +214,10 @@ class DavCollectionRepository @Inject constructor( /** Returns all collections that are both selected for synchronization and push-capable. */ fun getPushCapableAndSyncable(serviceId: Long) = dao.getPushCapableSyncCollections(serviceId) - suspend fun getPushRegisteredAndNotSyncable() = dao.getPushRegisteredAndNotSyncable() + suspend fun getPushRegistered(serviceId: Long) = dao.getPushRegistered(serviceId) + suspend fun getPushRegisteredAndNotSyncable(serviceId: Long) = dao.getPushRegisteredAndNotSyncable(serviceId) - fun getVapidKey(serviceId: Long) = dao.getFirstVapidKey(serviceId) + suspend fun getVapidKey(serviceId: Long) = dao.getFirstVapidKey(serviceId) /** * Inserts or updates the collection. diff --git a/app/src/main/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration18.kt b/app/src/main/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration18.kt index 32a0e200e3..a855460cbd 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration18.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration18.kt @@ -19,6 +19,7 @@ import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.components.SingletonComponent import dagger.multibindings.IntKey import dagger.multibindings.IntoMap +import kotlinx.coroutines.runBlocking import javax.inject.Inject /** @@ -39,17 +40,24 @@ class AccountSettingsMigration18 @Inject constructor( override fun migrate(account: Account) { val accountManager = AccountManager.get(context) - db.serviceDao().getByAccountAndType(account.name, Service.TYPE_CARDDAV)?.let { service -> - db.collectionDao().getByService(service.id).forEach { collection -> - // Find associated address book account by collection ID (if it exists) - val addressBookAccount = accountManager - .getAccountsByType(context.getString(R.string.account_type_address_book)) - .firstOrNull { accountManager.getUserData(it, LocalAddressBook.USER_DATA_COLLECTION_ID) == collection.id.toString() } + runBlocking { + db.serviceDao().getByAccountAndType(account.name, Service.TYPE_CARDDAV)?.let { service -> + db.collectionDao().getByService(service.id).forEach { collection -> + // Find associated address book account by collection ID (if it exists) + val addressBookAccount = accountManager + .getAccountsByType(context.getString(R.string.account_type_address_book)) + .firstOrNull { + accountManager.getUserData( + it, + LocalAddressBook.USER_DATA_COLLECTION_ID + ) == collection.id.toString() + } - if (addressBookAccount != null) { - // (Re-)assign address book to account - accountManager.setAndVerifyUserData(addressBookAccount, LocalAddressBook.USER_DATA_ACCOUNT_NAME, account.name) - accountManager.setAndVerifyUserData(addressBookAccount, LocalAddressBook.USER_DATA_ACCOUNT_TYPE, account.type) + if (addressBookAccount != null) { + // (Re-)assign address book to account + accountManager.setAndVerifyUserData(addressBookAccount, LocalAddressBook.USER_DATA_ACCOUNT_NAME, account.name) + accountManager.setAndVerifyUserData(addressBookAccount, LocalAddressBook.USER_DATA_ACCOUNT_TYPE, account.type) + } } } } From d93ed99371c1d387960adc9d6f07cf85fc860e5c Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Fri, 18 Apr 2025 10:07:30 +0200 Subject: [PATCH 10/20] Remove unused service repository dependency and update worker to suspend --- .../at/bitfire/davdroid/db/Collection.kt | 1 - .../davdroid/push/PushRegistrationManager.kt | 20 +++++++++---------- .../davdroid/push/PushRegistrationWorker.kt | 4 +--- .../repository/PreferenceRepository.kt | 3 --- 4 files changed, 11 insertions(+), 17 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/Collection.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/Collection.kt index c44485b2f5..65e77b1343 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/db/Collection.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/db/Collection.kt @@ -138,7 +138,6 @@ data class Collection( val supportsWebPush: Boolean = false, /** WebDAV-Push: VAPID public key */ - // TODO: add non-unique index val pushVapidKey: String? = null, /** WebDAV-Push subscription URL */ diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt index e11319a561..2df0d125e6 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt @@ -32,7 +32,6 @@ import at.bitfire.davdroid.repository.DavServiceRepository import dagger.Lazy import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.runBlocking import kotlinx.coroutines.runInterruptible import kotlinx.coroutines.withContext import okhttp3.HttpUrl @@ -92,7 +91,7 @@ class PushRegistrationManager @Inject constructor( else UnifiedPush.unregister(context, serviceId.toString()) - // UnifiedPush has now been called. It will do its work and then call back to UnifiedPushService, which + // UnifiedPush has now been called. It will do its work and then asynchronously call back to UnifiedPushService, which // will then call processSubscription or removeSubscription. } @@ -158,13 +157,13 @@ class PushRegistrationManager @Inject constructor( /** * Called when no subscription is available (anymore) for the given service. * - * Unsubscribes from all collections. + * Unsubscribes from all subscribed collections. */ - internal suspend fun removeSubscription(serviceId: Long) { - val service = serviceRepository.get(serviceId) ?: return + internal suspend fun removeSubscription(serviceId: Long) = withContext(dispatcher) { + val service = serviceRepository.get(serviceId) ?: return@withContext val unsubscribeFrom = collectionRepository.getPushRegistered(service.id) if (unsubscribeFrom.isEmpty()) - return + return@withContext val account = accountRepository.get().fromName(service.accountName) httpClientBuilder.get() @@ -268,10 +267,8 @@ class PushRegistrationManager @Inject constructor( * * Otherwise, a potentially existing worker is cancelled. */ - fun updatePeriodicWorker() { - val workerNeeded = runBlocking { - collectionRepository.anyPushCapable() - } + suspend fun updatePeriodicWorker() = withContext(dispatcher) { + val workerNeeded = collectionRepository.anyPushCapable() val workManager = WorkManager.getInstance(context) if (workerNeeded) { @@ -301,6 +298,9 @@ class PushRegistrationManager @Inject constructor( private const val WORKER_UNIQUE_NAME = "push-registration" const val WORKER_INTERVAL_DAYS = 1L + /** + * Single-thread dispatcher to synchronize tasks. + */ val dispatcher = Dispatchers.IO.limitedParallelism(1) } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt index 300aac586e..061452bbf4 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt @@ -8,7 +8,6 @@ import android.content.Context import androidx.hilt.work.HiltWorker import androidx.work.CoroutineWorker import androidx.work.WorkerParameters -import at.bitfire.davdroid.repository.DavServiceRepository import dagger.assisted.Assisted import dagger.assisted.AssistedInject import java.util.logging.Logger @@ -24,8 +23,7 @@ class PushRegistrationWorker @AssistedInject constructor( @Assisted context: Context, @Assisted workerParameters: WorkerParameters, private val logger: Logger, - private val pushRegistrationManager: PushRegistrationManager, - private val serviceRepository: DavServiceRepository + private val pushRegistrationManager: PushRegistrationManager ) : CoroutineWorker(context, workerParameters) { override suspend fun doWork(): Result { diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/PreferenceRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/PreferenceRepository.kt index c9c01405f3..e9efffdafa 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/repository/PreferenceRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/repository/PreferenceRepository.kt @@ -25,9 +25,6 @@ class PreferenceRepository @Inject constructor( companion object { const val LOG_TO_FILE = "log_to_file" - const val UNIFIED_PUSH_ENDPOINT_URL = "unified_push_endpoint_url" - const val UNIFIED_PUSH_ENDPOINT_KEY = "unified_push_endpoint_key" - const val UNIFIED_PUSH_ENDPOINT_AUTH = "unified_push_endpoint_auth" } private val preferences = PreferenceManager.getDefaultSharedPreferences(context) From 4865c9bac5a284ace7d8771b0f3bfe360bdcff86 Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Sun, 20 Apr 2025 13:39:31 +0200 Subject: [PATCH 11/20] Add suspend modifier to DAO methods and repository methods --- .../main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt | 2 +- app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt | 2 +- .../at/bitfire/davdroid/push/UnifiedPushService.kt | 3 ++- .../davdroid/repository/DavCollectionRepository.kt | 2 +- .../bitfire/davdroid/repository/DavServiceRepository.kt | 2 +- .../kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt | 9 ++++----- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt index cb859e4f6b..c8fd3eb395 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt @@ -76,7 +76,7 @@ interface CollectionDao { * pushTopic is available). */ @Query("SELECT * FROM collection WHERE serviceId=:serviceId AND sync AND supportsWebPush AND pushTopic IS NOT NULL") - fun getPushCapableSyncCollections(serviceId: Long): List + suspend fun getPushCapableSyncCollections(serviceId: Long): List @Query("SELECT * FROM collection WHERE serviceId=:serviceId AND pushSubscription IS NOT NULL") suspend fun getPushRegistered(serviceId: Long): List diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt index fd1639fccd..603a4b7d9f 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt @@ -26,7 +26,7 @@ interface ServiceDao { fun get(id: Long): Service? @Query("SELECT * FROM service") - fun getAll(): List + suspend fun getAll(): List @Insert(onConflict = OnConflictStrategy.REPLACE) fun insertOrReplace(service: Service): Long diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt index 3bebe6b800..a69470dbc5 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt @@ -13,6 +13,7 @@ import at.bitfire.davdroid.sync.TasksAppManager import at.bitfire.davdroid.sync.worker.SyncWorkerManager import dagger.Lazy import dagger.hilt.android.AndroidEntryPoint +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.runBlocking import org.unifiedpush.android.connector.FailedReason import org.unifiedpush.android.connector.PushService @@ -80,7 +81,7 @@ class UnifiedPushService : PushService() { } override fun onMessage(message: PushMessage, instance: String) { - runBlocking { + runBlocking(Dispatchers.Default) { if (!message.decrypted) { logger.severe("Received a push message that could not be decrypted.") return@runBlocking diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt index 61bf62dc8e..ad50c6a45f 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt @@ -212,7 +212,7 @@ class DavCollectionRepository @Inject constructor( fun getSyncTaskLists(serviceId: Long) = dao.getSyncTaskLists(serviceId) /** Returns all collections that are both selected for synchronization and push-capable. */ - fun getPushCapableAndSyncable(serviceId: Long) = dao.getPushCapableSyncCollections(serviceId) + suspend fun getPushCapableAndSyncable(serviceId: Long) = dao.getPushCapableSyncCollections(serviceId) suspend fun getPushRegistered(serviceId: Long) = dao.getPushRegistered(serviceId) suspend fun getPushRegisteredAndNotSyncable(serviceId: Long) = dao.getPushRegisteredAndNotSyncable(serviceId) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt index 0e2fd36b41..167b9f2947 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt @@ -20,7 +20,7 @@ class DavServiceRepository @Inject constructor( fun get(id: Long): Service? = dao.get(id) - fun getAll(): List = dao.getAll() + suspend fun getAll(): List = dao.getAll() fun getByAccountAndType(name: String, @ServiceType serviceType: String): Service? = dao.getByAccountAndType(name, serviceType) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt index a2f6c23466..f9c33e80fe 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt @@ -15,7 +15,6 @@ import androidx.lifecycle.viewModelScope import at.bitfire.cert4android.CustomCertStore import at.bitfire.davdroid.BuildConfig import at.bitfire.davdroid.push.PushRegistrationManager -import at.bitfire.davdroid.repository.DavCollectionRepository import at.bitfire.davdroid.repository.PreferenceRepository import at.bitfire.davdroid.settings.Settings import at.bitfire.davdroid.settings.SettingsManager @@ -39,7 +38,6 @@ import javax.inject.Inject @HiltViewModel class AppSettingsModel @Inject constructor( @ApplicationContext val context: Context, - private val collectionRepository: DavCollectionRepository, private val preferences: PreferenceRepository, private val pushRegistrationManager: PushRegistrationManager, private val settings: SettingsManager, @@ -50,9 +48,10 @@ class AppSettingsModel @Inject constructor( // debugging private val powerManager = context.getSystemService()!! - val batterySavingExempted = broadcastReceiverFlow(context, IntentFilter(PermissionUtils.ACTION_POWER_SAVE_WHITELIST_CHANGED), immediate = true) - .map { powerManager.isIgnoringBatteryOptimizations(BuildConfig.APPLICATION_ID) } - .stateIn(viewModelScope, SharingStarted.WhileSubscribed(), false) + val batterySavingExempted = + broadcastReceiverFlow(context, IntentFilter(PermissionUtils.ACTION_POWER_SAVE_WHITELIST_CHANGED), immediate = true) + .map { powerManager.isIgnoringBatteryOptimizations(BuildConfig.APPLICATION_ID) } + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(), false) fun verboseLogging() = preferences.logToFileFlow() fun updateVerboseLogging(verbose: Boolean) { From fe665f6fda291e8d2187ef56bc7402dbf4dc760a Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Mon, 21 Apr 2025 08:35:41 +0200 Subject: [PATCH 12/20] Add runBlocking to getByService call in CollectionListRefresherTest --- .../davdroid/servicedetection/CollectionListRefresherTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/CollectionListRefresherTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/CollectionListRefresherTest.kt index 53dbeb3003..db3450bf81 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/CollectionListRefresherTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/CollectionListRefresherTest.kt @@ -19,6 +19,7 @@ import dagger.hilt.android.testing.HiltAndroidTest import io.mockk.every import io.mockk.impl.annotations.MockK import io.mockk.junit4.MockKRule +import kotlinx.coroutines.runBlocking import okhttp3.mockwebserver.Dispatcher import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer @@ -142,7 +143,7 @@ class CollectionListRefresherTest { displayName = "My Contacts", description = "My Contacts Description" ), - db.collectionDao().getByService(service.id).first() + runBlocking { db.collectionDao().getByService(service.id).first() } ) } From 8d440bb275295e7cfc976b97c3e27e7151d7d61f Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Mon, 21 Apr 2025 08:41:53 +0200 Subject: [PATCH 13/20] Add documentation for UnifiedPushService and PushRegistrationManager --- .../davdroid/push/PushRegistrationManager.kt | 13 +++++++++---- .../at/bitfire/davdroid/push/UnifiedPushService.kt | 6 ++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt index 2df0d125e6..f6eb2e7fcc 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt @@ -48,6 +48,11 @@ import java.util.logging.Logger import javax.inject.Inject import javax.inject.Provider +/** + * Manages push registrations and subscriptions. + * + * To update push registrations and subscriptions (for instance after collections have been changed), call [update]. + */ class PushRegistrationManager @Inject constructor( private val accountRepository: Lazy, private val collectionRepository: DavCollectionRepository, @@ -73,13 +78,13 @@ class PushRegistrationManager @Inject constructor( /** * Same as [update], but for a specific database service. */ - suspend fun update(serviceId: Long) { + suspend fun update(serviceId: Long) = withContext(dispatcher) { updateService(serviceId) updatePeriodicWorker() } - private suspend fun updateService(serviceId: Long) = withContext(dispatcher) { - val service = serviceRepository.get(serviceId) ?: return@withContext + private suspend fun updateService(serviceId: Long) { + val service = serviceRepository.get(serviceId) ?: return val vapid = collectionRepository.getVapidKey(serviceId) if (vapid != null) @@ -299,7 +304,7 @@ class PushRegistrationManager @Inject constructor( const val WORKER_INTERVAL_DAYS = 1L /** - * Single-thread dispatcher to synchronize tasks. + * Single-thread dispatcher to synchronize non-private calls. */ val dispatcher = Dispatchers.IO.limitedParallelism(1) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt index a69470dbc5..10f322b459 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt @@ -23,6 +23,12 @@ import java.util.logging.Level import java.util.logging.Logger import javax.inject.Inject +/** + * Entry point for UnifiedPush. + * + * Calls [PushRegistrationManager] for most tasks, except incoming push messages, + * which are handled directly. + */ @AndroidEntryPoint class UnifiedPushService : PushService() { From 78a9680b9997505f85971c0e9bee32ef6d4f1cb4 Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Mon, 21 Apr 2025 08:48:05 +0200 Subject: [PATCH 14/20] Add fallback for push messages without topic --- .../at/bitfire/davdroid/push/UnifiedPushService.kt | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt index 10f322b459..83e1840c0f 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt @@ -129,10 +129,18 @@ class UnifiedPushService : PushService() { } } else { - logger.warning("Got push message without topic, syncing all accounts") - for (account in accountRepository.getAll()) + // fallback when no known topic is present (shouldn't happen) + val service = instance.toLongOrNull()?.let { serviceRepository.get(it) } + if (service != null) { + logger.warning("Got push message without topic and service, syncing all accounts") + val account = accountRepository.fromName(service.accountName) syncWorkerManager.enqueueOneTimeAllAuthorities(account, fromPush = true) + } else { + logger.warning("Got push message without topic, syncing all accounts") + for (account in accountRepository.getAll()) + syncWorkerManager.enqueueOneTimeAllAuthorities(account, fromPush = true) + } } } } From 57c64bf524ab3dca6fa9b3cce7d27a4ddf4a4cc8 Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Tue, 22 Apr 2025 14:25:48 +0200 Subject: [PATCH 15/20] [WIP] Add UnifiedPushService test with workaround for PushService binder --- app/build.gradle.kts | 4 + .../davdroid/push/UnifiedPushServiceTest.kt | 102 ++++++++++++++++++ .../davdroid/push/UnifiedPushService.kt | 40 ++++++- gradle/libs.versions.toml | 1 + 4 files changed, 143 insertions(+), 4 deletions(-) create mode 100644 app/src/androidTest/kotlin/at/bitfire/davdroid/push/UnifiedPushServiceTest.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index aa36d26afb..ce1dc6a316 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -137,6 +137,9 @@ dependencies { implementation(libs.kotlinx.coroutines) coreLibraryDesugaring(libs.android.desugaring) + // reflection + implementation(kotlin("reflect")) // only for UnifiedPushService.resetBinder workaround, remove when not needed anymore + // Hilt implementation(libs.hilt.android.base) ksp(libs.androidx.hilt.compiler) @@ -217,6 +220,7 @@ dependencies { androidTestImplementation(libs.androidx.work.testing) androidTestImplementation(libs.hilt.android.testing) androidTestImplementation(libs.junit) + androidTestImplementation(libs.kotlinx.coroutines.test) androidTestImplementation(libs.mockk.android) androidTestImplementation(libs.okhttp.mockwebserver) androidTestImplementation(libs.room.testing) diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/push/UnifiedPushServiceTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/push/UnifiedPushServiceTest.kt new file mode 100644 index 0000000000..9bb6b50a63 --- /dev/null +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/push/UnifiedPushServiceTest.kt @@ -0,0 +1,102 @@ +/* + * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details. + */ + +package at.bitfire.davdroid.push + +import android.content.Context +import android.content.Intent +import android.os.IBinder +import androidx.test.rule.ServiceTestRule +import dagger.hilt.android.qualifiers.ApplicationContext +import dagger.hilt.android.testing.BindValue +import dagger.hilt.android.testing.HiltAndroidRule +import dagger.hilt.android.testing.HiltAndroidTest +import io.mockk.coVerify +import io.mockk.confirmVerified +import io.mockk.every +import io.mockk.impl.annotations.RelaxedMockK +import io.mockk.junit4.MockKRule +import io.mockk.mockk +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.unifiedpush.android.connector.FailedReason +import org.unifiedpush.android.connector.PushService +import org.unifiedpush.android.connector.data.PushEndpoint +import javax.inject.Inject + +@HiltAndroidTest +class UnifiedPushServiceTest { + + @get:Rule + val hiltRule = HiltAndroidRule(this) + + @get:Rule + val mockKRule = MockKRule(this) + + @get:Rule + val serviceTestRule = ServiceTestRule() + + @Inject @ApplicationContext + lateinit var context: Context + + @RelaxedMockK @BindValue + lateinit var pushRegistrationManager: PushRegistrationManager + + lateinit var binder: IBinder + lateinit var unifiedPushService: UnifiedPushService + + + @Before + fun setUp() { + hiltRule.inject() + + binder = serviceTestRule.bindService(Intent(context, UnifiedPushService::class.java))!! + unifiedPushService = (binder as PushService.PushBinder).getService() as UnifiedPushService + } + + @After + fun tearDown() { + UnifiedPushService.resetBinder() + } + + + @Test + fun testOnNewEndpoint_1() { + val endpoint = mockk { + every { url } returns "https://example.com/12" + } + unifiedPushService.onNewEndpoint(endpoint, "12") + + coVerify { + pushRegistrationManager.processSubscription(12, endpoint) + } + confirmVerified(pushRegistrationManager) + } + + @Test + fun testOnNewEndpoint_2() { + val endpoint = mockk { + every { url } returns "https://example.com/34" + } + unifiedPushService.onNewEndpoint(endpoint, "34") + + coVerify { + pushRegistrationManager.processSubscription(34, endpoint) + } + confirmVerified(pushRegistrationManager) + } + + @Test + fun testOnRegistrationFailed() { + unifiedPushService.onRegistrationFailed(FailedReason.INTERNAL_ERROR, "34") + + coVerify { + pushRegistrationManager.removeSubscription(34) + } + confirmVerified(pushRegistrationManager) + } + +} \ No newline at end of file diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt index 83e1840c0f..13d29d90da 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt @@ -15,6 +15,7 @@ import dagger.Lazy import dagger.hilt.android.AndroidEntryPoint import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.runBlocking +import org.jetbrains.annotations.TestOnly import org.unifiedpush.android.connector.FailedReason import org.unifiedpush.android.connector.PushService import org.unifiedpush.android.connector.data.PushEndpoint @@ -22,6 +23,8 @@ import org.unifiedpush.android.connector.data.PushMessage import java.util.logging.Level import java.util.logging.Logger import javax.inject.Inject +import kotlin.reflect.KMutableProperty +import kotlin.reflect.jvm.isAccessible /** * Entry point for UnifiedPush. @@ -59,10 +62,10 @@ class UnifiedPushService : PushService() { override fun onNewEndpoint(endpoint: PushEndpoint, instance: String) { val serviceId = instance.toLongOrNull() ?: return - logger.log(Level.FINE, "Got UnifiedPush endpoint for service $serviceId", endpoint.url) + logger.warning("Got UnifiedPush endpoint for service $serviceId: ${endpoint.url}") // register new endpoint at CalDAV/CardDAV servers - runBlocking { + runBlocking(Dispatchers.Default) { pushRegistrationManager.processSubscription(serviceId, endpoint) } } @@ -72,7 +75,7 @@ class UnifiedPushService : PushService() { logger.warning("UnifiedPush registration failed for service $serviceId: $reason") // unregister subscriptions - runBlocking { + runBlocking(Dispatchers.Default) { pushRegistrationManager.removeSubscription(serviceId) } } @@ -81,7 +84,7 @@ class UnifiedPushService : PushService() { val serviceId = instance.toLongOrNull() ?: return logger.warning("UnifiedPush unregistered for service $serviceId") - runBlocking { + runBlocking(Dispatchers.Default) { pushRegistrationManager.removeSubscription(serviceId) } } @@ -145,4 +148,33 @@ class UnifiedPushService : PushService() { } } + + companion object { + + /** + * We need to reset PushService::Companion.binder to null before creating a new PushService with a new binder. The + * current implementation caches the binder, which will always use the first UnifiedPushService that is created during the first + * test. All following test will fail because the wrong binder is used by PushService. + * + * This method resets the binder using reflection, because it's not accessible directly. + * + * See https://codeberg.org/UnifiedPush/android-connector/issues/8 + */ + @TestOnly + fun resetBinder() { + // requires kotlin-reflection + val pushServiceClass = PushService::class + val companionClass = pushServiceClass.nestedClasses.first { it.isCompanion } + + val binderProperty = companionClass.members + .filterIsInstance(KMutableProperty::class.java) + .first { it.name.contains("binder") } + binderProperty.isAccessible = true + val binderSetter = binderProperty.setter + + binderSetter.call(companionClass, null) + } + + } + } \ No newline at end of file diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index c8c0bd7db3..41bb27383d 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -93,6 +93,7 @@ hilt-android-testing = { module = "com.google.dagger:hilt-android-testing", vers junit = { module = "junit:junit", version = "4.13.2" } kotlin-stdlib = { module = "org.jetbrains.kotlin:kotlin-stdlib", version.ref = "kotlin" } kotlinx-coroutines = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core", version.ref = "kotlinx-coroutines" } +kotlinx-coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", version.ref = "kotlinx-coroutines" } mikepenz-aboutLibraries = { module = "com.mikepenz:aboutlibraries-compose-m3", version.ref = "mikepenz-aboutLibraries" } mockk = { module = "io.mockk:mockk", version.ref = "mockk" } mockk-android = { module = "io.mockk:mockk-android", version.ref = "mockk" } From 5b6850588ad1ba59eb554d335c089510ed018625 Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Wed, 23 Apr 2025 10:43:13 +0200 Subject: [PATCH 16/20] Update UnifiedPush library version and clean up test code --- app/build.gradle.kts | 3 -- .../davdroid/push/UnifiedPushServiceTest.kt | 29 +++++++---------- .../davdroid/push/UnifiedPushService.kt | 32 ------------------- gradle/libs.versions.toml | 2 +- 4 files changed, 12 insertions(+), 54 deletions(-) diff --git a/app/build.gradle.kts b/app/build.gradle.kts index ce1dc6a316..2f2bc60e71 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -137,9 +137,6 @@ dependencies { implementation(libs.kotlinx.coroutines) coreLibraryDesugaring(libs.android.desugaring) - // reflection - implementation(kotlin("reflect")) // only for UnifiedPushService.resetBinder workaround, remove when not needed anymore - // Hilt implementation(libs.hilt.android.base) ksp(libs.androidx.hilt.compiler) diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/push/UnifiedPushServiceTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/push/UnifiedPushServiceTest.kt index 9bb6b50a63..0dd5c16104 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/push/UnifiedPushServiceTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/push/UnifiedPushServiceTest.kt @@ -18,7 +18,6 @@ import io.mockk.every import io.mockk.impl.annotations.RelaxedMockK import io.mockk.junit4.MockKRule import io.mockk.mockk -import org.junit.After import org.junit.Before import org.junit.Rule import org.junit.Test @@ -39,10 +38,12 @@ class UnifiedPushServiceTest { @get:Rule val serviceTestRule = ServiceTestRule() - @Inject @ApplicationContext + @Inject + @ApplicationContext lateinit var context: Context - @RelaxedMockK @BindValue + @RelaxedMockK + @BindValue lateinit var pushRegistrationManager: PushRegistrationManager lateinit var binder: IBinder @@ -57,14 +58,9 @@ class UnifiedPushServiceTest { unifiedPushService = (binder as PushService.PushBinder).getService() as UnifiedPushService } - @After - fun tearDown() { - UnifiedPushService.resetBinder() - } - @Test - fun testOnNewEndpoint_1() { + fun testOnNewEndpoint() { val endpoint = mockk { every { url } returns "https://example.com/12" } @@ -77,24 +73,21 @@ class UnifiedPushServiceTest { } @Test - fun testOnNewEndpoint_2() { - val endpoint = mockk { - every { url } returns "https://example.com/34" - } - unifiedPushService.onNewEndpoint(endpoint, "34") + fun testOnRegistrationFailed() { + unifiedPushService.onRegistrationFailed(FailedReason.INTERNAL_ERROR, "34") coVerify { - pushRegistrationManager.processSubscription(34, endpoint) + pushRegistrationManager.removeSubscription(34) } confirmVerified(pushRegistrationManager) } @Test - fun testOnRegistrationFailed() { - unifiedPushService.onRegistrationFailed(FailedReason.INTERNAL_ERROR, "34") + fun testOnUnregistered() { + unifiedPushService.onRegistrationFailed(FailedReason.INTERNAL_ERROR, "45") coVerify { - pushRegistrationManager.removeSubscription(34) + pushRegistrationManager.removeSubscription(45) } confirmVerified(pushRegistrationManager) } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt index 13d29d90da..f8390a6af0 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt @@ -15,7 +15,6 @@ import dagger.Lazy import dagger.hilt.android.AndroidEntryPoint import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.runBlocking -import org.jetbrains.annotations.TestOnly import org.unifiedpush.android.connector.FailedReason import org.unifiedpush.android.connector.PushService import org.unifiedpush.android.connector.data.PushEndpoint @@ -23,8 +22,6 @@ import org.unifiedpush.android.connector.data.PushMessage import java.util.logging.Level import java.util.logging.Logger import javax.inject.Inject -import kotlin.reflect.KMutableProperty -import kotlin.reflect.jvm.isAccessible /** * Entry point for UnifiedPush. @@ -148,33 +145,4 @@ class UnifiedPushService : PushService() { } } - - companion object { - - /** - * We need to reset PushService::Companion.binder to null before creating a new PushService with a new binder. The - * current implementation caches the binder, which will always use the first UnifiedPushService that is created during the first - * test. All following test will fail because the wrong binder is used by PushService. - * - * This method resets the binder using reflection, because it's not accessible directly. - * - * See https://codeberg.org/UnifiedPush/android-connector/issues/8 - */ - @TestOnly - fun resetBinder() { - // requires kotlin-reflection - val pushServiceClass = PushService::class - val companionClass = pushServiceClass.nestedClasses.first { it.isCompanion } - - val binderProperty = companionClass.members - .filterIsInstance(KMutableProperty::class.java) - .first { it.name.contains("binder") } - binderProperty.isAccessible = true - val binderSetter = binderProperty.setter - - binderSetter.call(companionClass, null) - } - - } - } \ No newline at end of file diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 41bb27383d..af4d588f68 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -39,7 +39,7 @@ mockk = "1.14.0" okhttp = "4.12.0" openid-appauth = "0.11.1" room = "2.7.0" -unifiedpush = "3.0.7" +unifiedpush = "3.0.8" unifiedpush-fcm = "3.0.0" # Other libraries, especially ical4android/ical4j, require Apache Commons. Some recent versions of Apache From d1945d7499f08b1b65f05a4f7fc4201962f4f4f8 Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Wed, 23 Apr 2025 15:43:00 +0200 Subject: [PATCH 17/20] Refactor push message handling, synchronization and coroutines --- .../davdroid/push/PushMessageHandlerTest.kt | 46 +++++++ .../at/bitfire/davdroid/db/CollectionDao.kt | 2 +- .../at/bitfire/davdroid/db/ServiceDao.kt | 3 + .../davdroid/push/PushMessageHandler.kt | 119 ++++++++++++++++++ .../davdroid/push/PushMessageParser.kt | 43 ------- .../davdroid/push/PushRegistrationManager.kt | 32 ++--- .../davdroid/push/UnifiedPushService.kt | 97 ++------------ .../repository/DavCollectionRepository.kt | 2 +- .../repository/DavServiceRepository.kt | 1 + .../davdroid/push/PushMessageParserTest.kt | 32 ----- 10 files changed, 197 insertions(+), 180 deletions(-) create mode 100644 app/src/androidTest/kotlin/at/bitfire/davdroid/push/PushMessageHandlerTest.kt create mode 100644 app/src/main/kotlin/at/bitfire/davdroid/push/PushMessageHandler.kt delete mode 100644 app/src/main/kotlin/at/bitfire/davdroid/push/PushMessageParser.kt delete mode 100644 app/src/test/kotlin/at/bitfire/davdroid/push/PushMessageParserTest.kt diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/push/PushMessageHandlerTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/push/PushMessageHandlerTest.kt new file mode 100644 index 0000000000..9d1549ea6e --- /dev/null +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/push/PushMessageHandlerTest.kt @@ -0,0 +1,46 @@ +/* + * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details. + */ + +package at.bitfire.davdroid.push + +import dagger.hilt.android.testing.HiltAndroidRule +import dagger.hilt.android.testing.HiltAndroidTest +import org.junit.Assert +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import javax.inject.Inject + +@HiltAndroidTest +class PushMessageHandlerTest { + + @get:Rule + val hiltRule = HiltAndroidRule(this) + + @Inject + lateinit var handler: PushMessageHandler + + @Before + fun setUp() { + hiltRule.inject() + } + + + @Test + fun testParse_InvalidXml() { + Assert.assertNull(handler.parse("Non-XML content")) + } + + @Test + fun testParse_WithXmlDeclAndTopic() { + val topic = handler.parse( + "" + + "" + + " O7M1nQ7cKkKTKsoS_j6Z3w" + + "" + ) + Assert.assertEquals("O7M1nQ7cKkKTKsoS_j6Z3w", topic) + } + +} \ No newline at end of file diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt index c8fd3eb395..39032d82b8 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt @@ -33,7 +33,7 @@ interface CollectionDao { fun getByServiceAndType(serviceId: Long, @CollectionType type: String): List @Query("SELECT * FROM collection WHERE pushTopic=:topic AND sync") - fun getSyncableByPushTopic(topic: String): Collection? + suspend fun getSyncableByPushTopic(topic: String): Collection? @Query("SELECT pushVapidKey FROM collection WHERE serviceId=:serviceId AND pushVapidKey IS NOT NULL LIMIT 1") suspend fun getFirstVapidKey(serviceId: Long): String? diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt index 603a4b7d9f..a9b205296e 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt @@ -25,6 +25,9 @@ interface ServiceDao { @Query("SELECT * FROM service WHERE id=:id") fun get(id: Long): Service? + @Query("SELECT * FROM service WHERE id=:id") + suspend fun getAsync(id: Long): Service? + @Query("SELECT * FROM service") suspend fun getAll(): List diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushMessageHandler.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushMessageHandler.kt new file mode 100644 index 0000000000..042cae926c --- /dev/null +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/PushMessageHandler.kt @@ -0,0 +1,119 @@ +/* + * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details. + */ + +package at.bitfire.davdroid.push + +import androidx.annotation.VisibleForTesting +import at.bitfire.dav4jvm.XmlReader +import at.bitfire.dav4jvm.XmlUtils +import at.bitfire.davdroid.db.Collection.Companion.TYPE_ADDRESSBOOK +import at.bitfire.davdroid.repository.AccountRepository +import at.bitfire.davdroid.repository.DavCollectionRepository +import at.bitfire.davdroid.repository.DavServiceRepository +import at.bitfire.davdroid.sync.SyncDataType +import at.bitfire.davdroid.sync.TasksAppManager +import at.bitfire.davdroid.sync.worker.SyncWorkerManager +import dagger.Lazy +import org.unifiedpush.android.connector.data.PushMessage +import org.xmlpull.v1.XmlPullParserException +import java.io.StringReader +import java.util.logging.Level +import java.util.logging.Logger +import javax.inject.Inject +import at.bitfire.dav4jvm.property.push.PushMessage as DavPushMessage + +/** + * Handles incoming WebDAV-Push messages. + */ +class PushMessageHandler @Inject constructor( + private val accountRepository: AccountRepository, + private val collectionRepository: DavCollectionRepository, + private val logger: Logger, + private val serviceRepository: DavServiceRepository, + private val syncWorkerManager: SyncWorkerManager, + private val tasksAppManager: Lazy +) { + + suspend fun processMessage(message: PushMessage, instance: String) { + if (!message.decrypted) { + logger.severe("Received a push message that could not be decrypted.") + return + } + val messageXml = message.content.toString(Charsets.UTF_8) + logger.log(Level.INFO, "Received push message", messageXml) + + // parse push notification + val topic = parse(messageXml) + + // sync affected collection + if (topic != null) { + logger.info("Got push notification for topic $topic") + + // Sync all authorities of account that the collection belongs to + // Later: only sync affected collection and authorities + collectionRepository.getSyncableByTopic(topic)?.let { collection -> + serviceRepository.getAsync(collection.serviceId)?.let { service -> + val syncDataTypes = mutableSetOf() + // If the type is an address book, add the contacts type + if (collection.type == TYPE_ADDRESSBOOK) + syncDataTypes += SyncDataType.CONTACTS + + // If the collection supports events, add the events type + if (collection.supportsVEVENT != false) + syncDataTypes += SyncDataType.EVENTS + + // If the collection supports tasks, make sure there's a provider installed, + // and add the tasks type + if (collection.supportsVJOURNAL != false || collection.supportsVTODO != false) + if (tasksAppManager.get().currentProvider() != null) + syncDataTypes += SyncDataType.TASKS + + // Schedule sync for all the types identified + val account = accountRepository.fromName(service.accountName) + for (syncDataType in syncDataTypes) + syncWorkerManager.enqueueOneTime(account, syncDataType, fromPush = true) + } + } + + } else { + // fallback when no known topic is present (shouldn't happen) + val service = instance.toLongOrNull()?.let { serviceRepository.get(it) } + if (service != null) { + logger.warning("Got push message without topic and service, syncing all accounts") + val account = accountRepository.fromName(service.accountName) + syncWorkerManager.enqueueOneTimeAllAuthorities(account, fromPush = true) + + } else { + logger.warning("Got push message without topic, syncing all accounts") + for (account in accountRepository.getAll()) + syncWorkerManager.enqueueOneTimeAllAuthorities(account, fromPush = true) + } + } + } + + /** + * 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 + */ + @VisibleForTesting + internal fun parse(message: String): String? { + var topic: String? = null + + val parser = XmlUtils.newPullParser() + try { + parser.setInput(StringReader(message)) + + XmlReader(parser).processTag(DavPushMessage.NAME) { + val pushMessage = DavPushMessage.Factory.create(parser) + topic = pushMessage.topic?.topic + } + } catch (e: XmlPullParserException) { + logger.log(Level.WARNING, "Couldn't parse push message", e) + } + + return topic + } + +} \ No newline at end of file diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushMessageParser.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushMessageParser.kt deleted file mode 100644 index a25cb0c97b..0000000000 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/PushMessageParser.kt +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details. - */ - -package at.bitfire.davdroid.push - -import at.bitfire.dav4jvm.XmlReader -import at.bitfire.dav4jvm.XmlUtils -import at.bitfire.dav4jvm.property.push.PushMessage -import org.xmlpull.v1.XmlPullParserException -import java.io.StringReader -import java.util.logging.Level -import java.util.logging.Logger -import javax.inject.Inject - -class PushMessageParser @Inject constructor( - private val logger: Logger -) { - - /** - * 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? { - var topic: String? = null - - val parser = XmlUtils.newPullParser() - try { - parser.setInput(StringReader(message)) - - XmlReader(parser).processTag(PushMessage.NAME) { - val pushMessage = PushMessage.Factory.create(parser) - topic = pushMessage.topic?.topic - } - } catch (e: XmlPullParserException) { - logger.log(Level.WARNING, "Couldn't parse push message", e) - } - - return topic - } - -} \ No newline at end of file diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt index f6eb2e7fcc..c4ad79bd95 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt @@ -26,14 +26,15 @@ import at.bitfire.dav4jvm.property.push.WebPushSubscription import at.bitfire.davdroid.db.Collection import at.bitfire.davdroid.db.Service import at.bitfire.davdroid.network.HttpClient +import at.bitfire.davdroid.push.PushRegistrationManager.Companion.mutex import at.bitfire.davdroid.repository.AccountRepository import at.bitfire.davdroid.repository.DavCollectionRepository import at.bitfire.davdroid.repository.DavServiceRepository import dagger.Lazy import dagger.hilt.android.qualifiers.ApplicationContext -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.runInterruptible -import kotlinx.coroutines.withContext +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import okhttp3.HttpUrl import okhttp3.HttpUrl.Companion.toHttpUrlOrNull import okhttp3.RequestBody.Companion.toRequestBody @@ -52,6 +53,8 @@ import javax.inject.Provider * Manages push registrations and subscriptions. * * To update push registrations and subscriptions (for instance after collections have been changed), call [update]. + * + * Public API calls are protected by [mutex] so that there won't be multiple subscribe/unsubscribe operations at the same time. */ class PushRegistrationManager @Inject constructor( private val accountRepository: Lazy, @@ -68,7 +71,7 @@ class PushRegistrationManager @Inject constructor( * * Also makes sure that the [PushRegistrationWorker] is enabled if there's a Push-enabled collection. */ - suspend fun update() = withContext(dispatcher) { + suspend fun update() = mutex.withLock { for (service in serviceRepository.getAll()) updateService(service.id) @@ -78,13 +81,13 @@ class PushRegistrationManager @Inject constructor( /** * Same as [update], but for a specific database service. */ - suspend fun update(serviceId: Long) = withContext(dispatcher) { + suspend fun update(serviceId: Long) = mutex.withLock { updateService(serviceId) updatePeriodicWorker() } private suspend fun updateService(serviceId: Long) { - val service = serviceRepository.get(serviceId) ?: return + val service = serviceRepository.getAsync(serviceId) ?: return val vapid = collectionRepository.getVapidKey(serviceId) if (vapid != null) @@ -102,12 +105,12 @@ class PushRegistrationManager @Inject constructor( /** - * Called when a subscription (endpoint) is available for the given service. + * Called by [UnifiedPushService] when a subscription (endpoint) is available for the given service. * * Uses the subscription to subscribe to syncable collections, and then unsubscribes from non-syncable collections. */ - internal suspend fun processSubscription(serviceId: Long, endpoint: PushEndpoint) = withContext(dispatcher) { - val service = serviceRepository.get(serviceId) ?: return@withContext + internal suspend fun processSubscription(serviceId: Long, endpoint: PushEndpoint) = mutex.withLock { + val service = serviceRepository.getAsync(serviceId) ?: return subscribeSyncable(service, endpoint) unsubscribeNotSyncable(service) @@ -164,11 +167,11 @@ class PushRegistrationManager @Inject constructor( * * Unsubscribes from all subscribed collections. */ - internal suspend fun removeSubscription(serviceId: Long) = withContext(dispatcher) { - val service = serviceRepository.get(serviceId) ?: return@withContext + internal suspend fun removeSubscription(serviceId: Long) = mutex.withLock { + val service = serviceRepository.getAsync(serviceId) ?: return val unsubscribeFrom = collectionRepository.getPushRegistered(service.id) if (unsubscribeFrom.isEmpty()) - return@withContext + return val account = accountRepository.get().fromName(service.accountName) httpClientBuilder.get() @@ -272,7 +275,7 @@ class PushRegistrationManager @Inject constructor( * * Otherwise, a potentially existing worker is cancelled. */ - suspend fun updatePeriodicWorker() = withContext(dispatcher) { + private suspend fun updatePeriodicWorker() { val workerNeeded = collectionRepository.anyPushCapable() val workManager = WorkManager.getInstance(context) @@ -303,10 +306,7 @@ class PushRegistrationManager @Inject constructor( private const val WORKER_UNIQUE_NAME = "push-registration" const val WORKER_INTERVAL_DAYS = 1L - /** - * Single-thread dispatcher to synchronize non-private calls. - */ - val dispatcher = Dispatchers.IO.limitedParallelism(1) + val mutex = Mutex() } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt index f8390a6af0..fb182651ca 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt @@ -4,22 +4,13 @@ package at.bitfire.davdroid.push -import at.bitfire.davdroid.db.Collection.Companion.TYPE_ADDRESSBOOK -import at.bitfire.davdroid.repository.AccountRepository -import at.bitfire.davdroid.repository.DavCollectionRepository -import at.bitfire.davdroid.repository.DavServiceRepository -import at.bitfire.davdroid.sync.SyncDataType -import at.bitfire.davdroid.sync.TasksAppManager -import at.bitfire.davdroid.sync.worker.SyncWorkerManager import dagger.Lazy import dagger.hilt.android.AndroidEntryPoint -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.runBlocking import org.unifiedpush.android.connector.FailedReason import org.unifiedpush.android.connector.PushService import org.unifiedpush.android.connector.data.PushEndpoint import org.unifiedpush.android.connector.data.PushMessage -import java.util.logging.Level import java.util.logging.Logger import javax.inject.Inject @@ -32,29 +23,14 @@ import javax.inject.Inject @AndroidEntryPoint class UnifiedPushService : PushService() { - @Inject - lateinit var accountRepository: AccountRepository - - @Inject - lateinit var collectionRepository: DavCollectionRepository - @Inject lateinit var logger: Logger @Inject - lateinit var serviceRepository: DavServiceRepository - - @Inject - lateinit var parsePushMessage: PushMessageParser - - @Inject - lateinit var pushRegistrationManager: PushRegistrationManager - - @Inject - lateinit var tasksAppManager: Lazy + lateinit var pushMessageHandler: Lazy @Inject - lateinit var syncWorkerManager: SyncWorkerManager + lateinit var pushRegistrationManager: Lazy override fun onNewEndpoint(endpoint: PushEndpoint, instance: String) { @@ -62,8 +38,8 @@ class UnifiedPushService : PushService() { logger.warning("Got UnifiedPush endpoint for service $serviceId: ${endpoint.url}") // register new endpoint at CalDAV/CardDAV servers - runBlocking(Dispatchers.Default) { - pushRegistrationManager.processSubscription(serviceId, endpoint) + runBlocking { + pushRegistrationManager.get().processSubscription(serviceId, endpoint) } } @@ -72,8 +48,8 @@ class UnifiedPushService : PushService() { logger.warning("UnifiedPush registration failed for service $serviceId: $reason") // unregister subscriptions - runBlocking(Dispatchers.Default) { - pushRegistrationManager.removeSubscription(serviceId) + runBlocking { + pushRegistrationManager.get().removeSubscription(serviceId) } } @@ -81,67 +57,14 @@ class UnifiedPushService : PushService() { val serviceId = instance.toLongOrNull() ?: return logger.warning("UnifiedPush unregistered for service $serviceId") - runBlocking(Dispatchers.Default) { - pushRegistrationManager.removeSubscription(serviceId) + runBlocking { + pushRegistrationManager.get().removeSubscription(serviceId) } } override fun onMessage(message: PushMessage, instance: String) { - runBlocking(Dispatchers.Default) { - if (!message.decrypted) { - logger.severe("Received a push message that could not be decrypted.") - return@runBlocking - } - val messageXml = message.content.toString(Charsets.UTF_8) - logger.log(Level.INFO, "Received push message", messageXml) - - // parse push notification - val topic = parsePushMessage(messageXml) - - // sync affected collection - if (topic != null) { - logger.info("Got push notification for topic $topic") - - // Sync all authorities of account that the collection belongs to - // Later: only sync affected collection and authorities - collectionRepository.getSyncableByTopic(topic)?.let { collection -> - serviceRepository.get(collection.serviceId)?.let { service -> - val syncDataTypes = mutableSetOf() - // If the type is an address book, add the contacts type - if (collection.type == TYPE_ADDRESSBOOK) - syncDataTypes += SyncDataType.CONTACTS - - // If the collection supports events, add the events type - if (collection.supportsVEVENT != false) - syncDataTypes += SyncDataType.EVENTS - - // If the collection supports tasks, make sure there's a provider installed, - // and add the tasks type - if (collection.supportsVJOURNAL != false || collection.supportsVTODO != false) - if (tasksAppManager.get().currentProvider() != null) - syncDataTypes += SyncDataType.TASKS - - // Schedule sync for all the types identified - val account = accountRepository.fromName(service.accountName) - for (syncDataType in syncDataTypes) - syncWorkerManager.enqueueOneTime(account, syncDataType, fromPush = true) - } - } - - } else { - // fallback when no known topic is present (shouldn't happen) - val service = instance.toLongOrNull()?.let { serviceRepository.get(it) } - if (service != null) { - logger.warning("Got push message without topic and service, syncing all accounts") - val account = accountRepository.fromName(service.accountName) - syncWorkerManager.enqueueOneTimeAllAuthorities(account, fromPush = true) - - } else { - logger.warning("Got push message without topic, syncing all accounts") - for (account in accountRepository.getAll()) - syncWorkerManager.enqueueOneTimeAllAuthorities(account, fromPush = true) - } - } + runBlocking { + pushMessageHandler.get().processMessage(message, instance) } } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt index ad50c6a45f..5aebdce1e2 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt @@ -193,7 +193,7 @@ class DavCollectionRepository @Inject constructor( } } - fun getSyncableByTopic(topic: String) = dao.getSyncableByPushTopic(topic) + suspend fun getSyncableByTopic(topic: String) = dao.getSyncableByPushTopic(topic) fun get(id: Long) = dao.get(id) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt index 167b9f2947..8556d10b12 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt @@ -19,6 +19,7 @@ class DavServiceRepository @Inject constructor( // Read fun get(id: Long): Service? = dao.get(id) + suspend fun getAsync(id: Long): Service? = dao.getAsync(id) suspend fun getAll(): List = dao.getAll() diff --git a/app/src/test/kotlin/at/bitfire/davdroid/push/PushMessageParserTest.kt b/app/src/test/kotlin/at/bitfire/davdroid/push/PushMessageParserTest.kt deleted file mode 100644 index 22c72b11d8..0000000000 --- a/app/src/test/kotlin/at/bitfire/davdroid/push/PushMessageParserTest.kt +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details. - */ - -package at.bitfire.davdroid.push - -import org.junit.Assert.assertEquals -import org.junit.Assert.assertNull -import org.junit.Test -import java.util.logging.Logger - -class PushMessageParserTest { - - private val parse = PushMessageParser(logger = Logger.getGlobal()) - - @Test - fun testInvalidXml() { - assertNull(parse("Non-XML content")) - } - - @Test - fun testWithXmlDeclAndTopic() { - val topic = parse( - "" + - "" + - " O7M1nQ7cKkKTKsoS_j6Z3w" + - "" - ) - assertEquals("O7M1nQ7cKkKTKsoS_j6Z3w", topic) - } - -} \ No newline at end of file From 5f801a00568d2a9905b99f3db9ab7500673210d0 Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Wed, 23 Apr 2025 15:43:00 +0200 Subject: [PATCH 18/20] Add coroutine dispatchers for push registration and unregistration --- .../davdroid/push/PushMessageHandlerTest.kt | 46 +++++++ .../at/bitfire/davdroid/db/CollectionDao.kt | 2 +- .../at/bitfire/davdroid/db/ServiceDao.kt | 3 + .../davdroid/push/PushMessageHandler.kt | 119 ++++++++++++++++ .../davdroid/push/PushMessageParser.kt | 43 ------ .../davdroid/push/PushRegistrationManager.kt | 130 ++++++++++-------- .../davdroid/push/UnifiedPushService.kt | 97 ++----------- .../repository/DavCollectionRepository.kt | 2 +- .../repository/DavServiceRepository.kt | 1 + .../util/CoroutineDispatcherModule.kt | 43 ++++++ .../davdroid/push/PushMessageParserTest.kt | 32 ----- 11 files changed, 294 insertions(+), 224 deletions(-) create mode 100644 app/src/androidTest/kotlin/at/bitfire/davdroid/push/PushMessageHandlerTest.kt create mode 100644 app/src/main/kotlin/at/bitfire/davdroid/push/PushMessageHandler.kt delete mode 100644 app/src/main/kotlin/at/bitfire/davdroid/push/PushMessageParser.kt create mode 100644 app/src/main/kotlin/at/bitfire/davdroid/util/CoroutineDispatcherModule.kt delete mode 100644 app/src/test/kotlin/at/bitfire/davdroid/push/PushMessageParserTest.kt diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/push/PushMessageHandlerTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/push/PushMessageHandlerTest.kt new file mode 100644 index 0000000000..9d1549ea6e --- /dev/null +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/push/PushMessageHandlerTest.kt @@ -0,0 +1,46 @@ +/* + * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details. + */ + +package at.bitfire.davdroid.push + +import dagger.hilt.android.testing.HiltAndroidRule +import dagger.hilt.android.testing.HiltAndroidTest +import org.junit.Assert +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import javax.inject.Inject + +@HiltAndroidTest +class PushMessageHandlerTest { + + @get:Rule + val hiltRule = HiltAndroidRule(this) + + @Inject + lateinit var handler: PushMessageHandler + + @Before + fun setUp() { + hiltRule.inject() + } + + + @Test + fun testParse_InvalidXml() { + Assert.assertNull(handler.parse("Non-XML content")) + } + + @Test + fun testParse_WithXmlDeclAndTopic() { + val topic = handler.parse( + "" + + "" + + " O7M1nQ7cKkKTKsoS_j6Z3w" + + "" + ) + Assert.assertEquals("O7M1nQ7cKkKTKsoS_j6Z3w", topic) + } + +} \ No newline at end of file diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt index c8fd3eb395..39032d82b8 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt @@ -33,7 +33,7 @@ interface CollectionDao { fun getByServiceAndType(serviceId: Long, @CollectionType type: String): List @Query("SELECT * FROM collection WHERE pushTopic=:topic AND sync") - fun getSyncableByPushTopic(topic: String): Collection? + suspend fun getSyncableByPushTopic(topic: String): Collection? @Query("SELECT pushVapidKey FROM collection WHERE serviceId=:serviceId AND pushVapidKey IS NOT NULL LIMIT 1") suspend fun getFirstVapidKey(serviceId: Long): String? diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt index 603a4b7d9f..a9b205296e 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt @@ -25,6 +25,9 @@ interface ServiceDao { @Query("SELECT * FROM service WHERE id=:id") fun get(id: Long): Service? + @Query("SELECT * FROM service WHERE id=:id") + suspend fun getAsync(id: Long): Service? + @Query("SELECT * FROM service") suspend fun getAll(): List diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushMessageHandler.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushMessageHandler.kt new file mode 100644 index 0000000000..042cae926c --- /dev/null +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/PushMessageHandler.kt @@ -0,0 +1,119 @@ +/* + * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details. + */ + +package at.bitfire.davdroid.push + +import androidx.annotation.VisibleForTesting +import at.bitfire.dav4jvm.XmlReader +import at.bitfire.dav4jvm.XmlUtils +import at.bitfire.davdroid.db.Collection.Companion.TYPE_ADDRESSBOOK +import at.bitfire.davdroid.repository.AccountRepository +import at.bitfire.davdroid.repository.DavCollectionRepository +import at.bitfire.davdroid.repository.DavServiceRepository +import at.bitfire.davdroid.sync.SyncDataType +import at.bitfire.davdroid.sync.TasksAppManager +import at.bitfire.davdroid.sync.worker.SyncWorkerManager +import dagger.Lazy +import org.unifiedpush.android.connector.data.PushMessage +import org.xmlpull.v1.XmlPullParserException +import java.io.StringReader +import java.util.logging.Level +import java.util.logging.Logger +import javax.inject.Inject +import at.bitfire.dav4jvm.property.push.PushMessage as DavPushMessage + +/** + * Handles incoming WebDAV-Push messages. + */ +class PushMessageHandler @Inject constructor( + private val accountRepository: AccountRepository, + private val collectionRepository: DavCollectionRepository, + private val logger: Logger, + private val serviceRepository: DavServiceRepository, + private val syncWorkerManager: SyncWorkerManager, + private val tasksAppManager: Lazy +) { + + suspend fun processMessage(message: PushMessage, instance: String) { + if (!message.decrypted) { + logger.severe("Received a push message that could not be decrypted.") + return + } + val messageXml = message.content.toString(Charsets.UTF_8) + logger.log(Level.INFO, "Received push message", messageXml) + + // parse push notification + val topic = parse(messageXml) + + // sync affected collection + if (topic != null) { + logger.info("Got push notification for topic $topic") + + // Sync all authorities of account that the collection belongs to + // Later: only sync affected collection and authorities + collectionRepository.getSyncableByTopic(topic)?.let { collection -> + serviceRepository.getAsync(collection.serviceId)?.let { service -> + val syncDataTypes = mutableSetOf() + // If the type is an address book, add the contacts type + if (collection.type == TYPE_ADDRESSBOOK) + syncDataTypes += SyncDataType.CONTACTS + + // If the collection supports events, add the events type + if (collection.supportsVEVENT != false) + syncDataTypes += SyncDataType.EVENTS + + // If the collection supports tasks, make sure there's a provider installed, + // and add the tasks type + if (collection.supportsVJOURNAL != false || collection.supportsVTODO != false) + if (tasksAppManager.get().currentProvider() != null) + syncDataTypes += SyncDataType.TASKS + + // Schedule sync for all the types identified + val account = accountRepository.fromName(service.accountName) + for (syncDataType in syncDataTypes) + syncWorkerManager.enqueueOneTime(account, syncDataType, fromPush = true) + } + } + + } else { + // fallback when no known topic is present (shouldn't happen) + val service = instance.toLongOrNull()?.let { serviceRepository.get(it) } + if (service != null) { + logger.warning("Got push message without topic and service, syncing all accounts") + val account = accountRepository.fromName(service.accountName) + syncWorkerManager.enqueueOneTimeAllAuthorities(account, fromPush = true) + + } else { + logger.warning("Got push message without topic, syncing all accounts") + for (account in accountRepository.getAll()) + syncWorkerManager.enqueueOneTimeAllAuthorities(account, fromPush = true) + } + } + } + + /** + * 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 + */ + @VisibleForTesting + internal fun parse(message: String): String? { + var topic: String? = null + + val parser = XmlUtils.newPullParser() + try { + parser.setInput(StringReader(message)) + + XmlReader(parser).processTag(DavPushMessage.NAME) { + val pushMessage = DavPushMessage.Factory.create(parser) + topic = pushMessage.topic?.topic + } + } catch (e: XmlPullParserException) { + logger.log(Level.WARNING, "Couldn't parse push message", e) + } + + return topic + } + +} \ No newline at end of file diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushMessageParser.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushMessageParser.kt deleted file mode 100644 index a25cb0c97b..0000000000 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/PushMessageParser.kt +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details. - */ - -package at.bitfire.davdroid.push - -import at.bitfire.dav4jvm.XmlReader -import at.bitfire.dav4jvm.XmlUtils -import at.bitfire.dav4jvm.property.push.PushMessage -import org.xmlpull.v1.XmlPullParserException -import java.io.StringReader -import java.util.logging.Level -import java.util.logging.Logger -import javax.inject.Inject - -class PushMessageParser @Inject constructor( - private val logger: Logger -) { - - /** - * 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? { - var topic: String? = null - - val parser = XmlUtils.newPullParser() - try { - parser.setInput(StringReader(message)) - - XmlReader(parser).processTag(PushMessage.NAME) { - val pushMessage = PushMessage.Factory.create(parser) - topic = pushMessage.topic?.topic - } - } catch (e: XmlPullParserException) { - logger.log(Level.WARNING, "Couldn't parse push message", e) - } - - return topic - } - -} \ No newline at end of file diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt index f6eb2e7fcc..ed79e6ead5 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt @@ -26,13 +26,17 @@ import at.bitfire.dav4jvm.property.push.WebPushSubscription import at.bitfire.davdroid.db.Collection import at.bitfire.davdroid.db.Service import at.bitfire.davdroid.network.HttpClient +import at.bitfire.davdroid.push.PushRegistrationManager.Companion.mutex import at.bitfire.davdroid.repository.AccountRepository import at.bitfire.davdroid.repository.DavCollectionRepository import at.bitfire.davdroid.repository.DavServiceRepository +import at.bitfire.davdroid.util.IoDispatcher import dagger.Lazy import dagger.hilt.android.qualifiers.ApplicationContext -import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.runInterruptible +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import okhttp3.HttpUrl import okhttp3.HttpUrl.Companion.toHttpUrlOrNull @@ -52,12 +56,15 @@ import javax.inject.Provider * Manages push registrations and subscriptions. * * To update push registrations and subscriptions (for instance after collections have been changed), call [update]. + * + * Public API calls are protected by [mutex] so that there won't be multiple subscribe/unsubscribe operations at the same time. */ class PushRegistrationManager @Inject constructor( private val accountRepository: Lazy, private val collectionRepository: DavCollectionRepository, @ApplicationContext private val context: Context, private val httpClientBuilder: Provider, + @IoDispatcher private val ioDispatcher: CoroutineDispatcher, private val logger: Logger, private val serviceRepository: DavServiceRepository ) { @@ -68,7 +75,7 @@ class PushRegistrationManager @Inject constructor( * * Also makes sure that the [PushRegistrationWorker] is enabled if there's a Push-enabled collection. */ - suspend fun update() = withContext(dispatcher) { + suspend fun update() = mutex.withLock { for (service in serviceRepository.getAll()) updateService(service.id) @@ -78,13 +85,13 @@ class PushRegistrationManager @Inject constructor( /** * Same as [update], but for a specific database service. */ - suspend fun update(serviceId: Long) = withContext(dispatcher) { + suspend fun update(serviceId: Long) = mutex.withLock { updateService(serviceId) updatePeriodicWorker() } private suspend fun updateService(serviceId: Long) { - val service = serviceRepository.get(serviceId) ?: return + val service = serviceRepository.getAsync(serviceId) ?: return val vapid = collectionRepository.getVapidKey(serviceId) if (vapid != null) @@ -102,12 +109,12 @@ class PushRegistrationManager @Inject constructor( /** - * Called when a subscription (endpoint) is available for the given service. + * Called by [UnifiedPushService] when a subscription (endpoint) is available for the given service. * * Uses the subscription to subscribe to syncable collections, and then unsubscribes from non-syncable collections. */ - internal suspend fun processSubscription(serviceId: Long, endpoint: PushEndpoint) = withContext(dispatcher) { - val service = serviceRepository.get(serviceId) ?: return@withContext + internal suspend fun processSubscription(serviceId: Long, endpoint: PushEndpoint) = mutex.withLock { + val service = serviceRepository.getAsync(serviceId) ?: return subscribeSyncable(service, endpoint) unsubscribeNotSyncable(service) @@ -118,27 +125,29 @@ class PushRegistrationManager @Inject constructor( if (subscribeTo.isEmpty()) return - val account = accountRepository.get().fromName(service.accountName) - httpClientBuilder.get() - .fromAccount(account) - .build() - .use { httpClient -> - for (collection in subscribeTo) - try { - val expires = collection.pushSubscriptionExpires - // calculate next run time, but use the duplicate interval for safety (times are not exact) - val nextRun = Instant.now() + Duration.ofDays(2 * WORKER_INTERVAL_DAYS) - if (expires != null && expires >= nextRun.epochSecond) - logger.fine("Push subscription for ${collection.url} is still valid until ${collection.pushSubscriptionExpires}") - else { - // no existing subscription or expiring soon - logger.fine("Registering push subscription for ${collection.url}") - subscribe(httpClient, collection, endpoint) + withContext(ioDispatcher) { // HttpClientBuilder.fromAccount() must not run in main thread and is not suspending + val account = accountRepository.get().fromName(service.accountName) + httpClientBuilder.get() + .fromAccount(account) + .build() + .use { httpClient -> + for (collection in subscribeTo) + try { + val expires = collection.pushSubscriptionExpires + // calculate next run time, but use the duplicate interval for safety (times are not exact) + val nextRun = Instant.now() + Duration.ofDays(2 * WORKER_INTERVAL_DAYS) + if (expires != null && expires >= nextRun.epochSecond) + logger.fine("Push subscription for ${collection.url} is still valid until ${collection.pushSubscriptionExpires}") + else { + // no existing subscription or expiring soon + logger.fine("Registering push subscription for ${collection.url}") + subscribe(httpClient, collection, endpoint) + } + } catch (e: Exception) { + logger.log(Level.WARNING, "Couldn't register subscription at CalDAV/CardDAV server", e) } - } catch (e: Exception) { - logger.log(Level.WARNING, "Couldn't register subscription at CalDAV/CardDAV server", e) - } - } + } + } } private suspend fun unsubscribeNotSyncable(service: Service) { @@ -146,17 +155,19 @@ class PushRegistrationManager @Inject constructor( if (unsubscribeFrom.isEmpty()) return - val account = accountRepository.get().fromName(service.accountName) - httpClientBuilder.get() - .fromAccount(account) - .build() - .use { httpClient -> - for (collection in unsubscribeFrom) - collection.pushSubscription?.toHttpUrlOrNull()?.let { url -> - logger.info("Unregistering push for ${collection.url}") - unsubscribe(httpClient, collection, url) - } - } + withContext(ioDispatcher) { // HttpClientBuilder.fromAccount() must not run in main thread and is not suspending + val account = accountRepository.get().fromName(service.accountName) + httpClientBuilder.get() + .fromAccount(account) + .build() + .use { httpClient -> + for (collection in unsubscribeFrom) + collection.pushSubscription?.toHttpUrlOrNull()?.let { url -> + logger.info("Unregistering push for ${collection.url}") + unsubscribe(httpClient, collection, url) + } + } + } } /** @@ -164,23 +175,25 @@ class PushRegistrationManager @Inject constructor( * * Unsubscribes from all subscribed collections. */ - internal suspend fun removeSubscription(serviceId: Long) = withContext(dispatcher) { - val service = serviceRepository.get(serviceId) ?: return@withContext + internal suspend fun removeSubscription(serviceId: Long) = mutex.withLock { + val service = serviceRepository.getAsync(serviceId) ?: return val unsubscribeFrom = collectionRepository.getPushRegistered(service.id) if (unsubscribeFrom.isEmpty()) - return@withContext - - val account = accountRepository.get().fromName(service.accountName) - httpClientBuilder.get() - .fromAccount(account) - .build() - .use { httpClient -> - for (collection in unsubscribeFrom) - collection.pushSubscription?.toHttpUrlOrNull()?.let { url -> - logger.info("Unregistering push for ${collection.url}") - unsubscribe(httpClient, collection, url) - } - } + return + + withContext(ioDispatcher) { // HttpClientBuilder.fromAccount() must not run in main thread and is not suspending + val account = accountRepository.get().fromName(service.accountName) + httpClientBuilder.get() + .fromAccount(account) + .build() + .use { httpClient -> + for (collection in unsubscribeFrom) + collection.pushSubscription?.toHttpUrlOrNull()?.let { url -> + logger.info("Unregistering push for ${collection.url}") + unsubscribe(httpClient, collection, url) + } + } + } } @@ -224,7 +237,7 @@ class PushRegistrationManager @Inject constructor( } serializer.endDocument() - runInterruptible { + runInterruptible(ioDispatcher) { val xml = writer.toString().toRequestBody(DavResource.MIME_XML) DavCollection(httpClient.okHttpClient, collection.url).post(xml) { response -> if (response.isSuccessful) { @@ -245,7 +258,7 @@ class PushRegistrationManager @Inject constructor( } private suspend fun unsubscribe(httpClient: HttpClient, collection: Collection, url: HttpUrl) { - runInterruptible { + runInterruptible(ioDispatcher) { try { DavResource(httpClient.okHttpClient, url).delete { // deleted @@ -272,7 +285,7 @@ class PushRegistrationManager @Inject constructor( * * Otherwise, a potentially existing worker is cancelled. */ - suspend fun updatePeriodicWorker() = withContext(dispatcher) { + private suspend fun updatePeriodicWorker() { val workerNeeded = collectionRepository.anyPushCapable() val workManager = WorkManager.getInstance(context) @@ -303,10 +316,7 @@ class PushRegistrationManager @Inject constructor( private const val WORKER_UNIQUE_NAME = "push-registration" const val WORKER_INTERVAL_DAYS = 1L - /** - * Single-thread dispatcher to synchronize non-private calls. - */ - val dispatcher = Dispatchers.IO.limitedParallelism(1) + val mutex = Mutex() } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt index f8390a6af0..fb182651ca 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt @@ -4,22 +4,13 @@ package at.bitfire.davdroid.push -import at.bitfire.davdroid.db.Collection.Companion.TYPE_ADDRESSBOOK -import at.bitfire.davdroid.repository.AccountRepository -import at.bitfire.davdroid.repository.DavCollectionRepository -import at.bitfire.davdroid.repository.DavServiceRepository -import at.bitfire.davdroid.sync.SyncDataType -import at.bitfire.davdroid.sync.TasksAppManager -import at.bitfire.davdroid.sync.worker.SyncWorkerManager import dagger.Lazy import dagger.hilt.android.AndroidEntryPoint -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.runBlocking import org.unifiedpush.android.connector.FailedReason import org.unifiedpush.android.connector.PushService import org.unifiedpush.android.connector.data.PushEndpoint import org.unifiedpush.android.connector.data.PushMessage -import java.util.logging.Level import java.util.logging.Logger import javax.inject.Inject @@ -32,29 +23,14 @@ import javax.inject.Inject @AndroidEntryPoint class UnifiedPushService : PushService() { - @Inject - lateinit var accountRepository: AccountRepository - - @Inject - lateinit var collectionRepository: DavCollectionRepository - @Inject lateinit var logger: Logger @Inject - lateinit var serviceRepository: DavServiceRepository - - @Inject - lateinit var parsePushMessage: PushMessageParser - - @Inject - lateinit var pushRegistrationManager: PushRegistrationManager - - @Inject - lateinit var tasksAppManager: Lazy + lateinit var pushMessageHandler: Lazy @Inject - lateinit var syncWorkerManager: SyncWorkerManager + lateinit var pushRegistrationManager: Lazy override fun onNewEndpoint(endpoint: PushEndpoint, instance: String) { @@ -62,8 +38,8 @@ class UnifiedPushService : PushService() { logger.warning("Got UnifiedPush endpoint for service $serviceId: ${endpoint.url}") // register new endpoint at CalDAV/CardDAV servers - runBlocking(Dispatchers.Default) { - pushRegistrationManager.processSubscription(serviceId, endpoint) + runBlocking { + pushRegistrationManager.get().processSubscription(serviceId, endpoint) } } @@ -72,8 +48,8 @@ class UnifiedPushService : PushService() { logger.warning("UnifiedPush registration failed for service $serviceId: $reason") // unregister subscriptions - runBlocking(Dispatchers.Default) { - pushRegistrationManager.removeSubscription(serviceId) + runBlocking { + pushRegistrationManager.get().removeSubscription(serviceId) } } @@ -81,67 +57,14 @@ class UnifiedPushService : PushService() { val serviceId = instance.toLongOrNull() ?: return logger.warning("UnifiedPush unregistered for service $serviceId") - runBlocking(Dispatchers.Default) { - pushRegistrationManager.removeSubscription(serviceId) + runBlocking { + pushRegistrationManager.get().removeSubscription(serviceId) } } override fun onMessage(message: PushMessage, instance: String) { - runBlocking(Dispatchers.Default) { - if (!message.decrypted) { - logger.severe("Received a push message that could not be decrypted.") - return@runBlocking - } - val messageXml = message.content.toString(Charsets.UTF_8) - logger.log(Level.INFO, "Received push message", messageXml) - - // parse push notification - val topic = parsePushMessage(messageXml) - - // sync affected collection - if (topic != null) { - logger.info("Got push notification for topic $topic") - - // Sync all authorities of account that the collection belongs to - // Later: only sync affected collection and authorities - collectionRepository.getSyncableByTopic(topic)?.let { collection -> - serviceRepository.get(collection.serviceId)?.let { service -> - val syncDataTypes = mutableSetOf() - // If the type is an address book, add the contacts type - if (collection.type == TYPE_ADDRESSBOOK) - syncDataTypes += SyncDataType.CONTACTS - - // If the collection supports events, add the events type - if (collection.supportsVEVENT != false) - syncDataTypes += SyncDataType.EVENTS - - // If the collection supports tasks, make sure there's a provider installed, - // and add the tasks type - if (collection.supportsVJOURNAL != false || collection.supportsVTODO != false) - if (tasksAppManager.get().currentProvider() != null) - syncDataTypes += SyncDataType.TASKS - - // Schedule sync for all the types identified - val account = accountRepository.fromName(service.accountName) - for (syncDataType in syncDataTypes) - syncWorkerManager.enqueueOneTime(account, syncDataType, fromPush = true) - } - } - - } else { - // fallback when no known topic is present (shouldn't happen) - val service = instance.toLongOrNull()?.let { serviceRepository.get(it) } - if (service != null) { - logger.warning("Got push message without topic and service, syncing all accounts") - val account = accountRepository.fromName(service.accountName) - syncWorkerManager.enqueueOneTimeAllAuthorities(account, fromPush = true) - - } else { - logger.warning("Got push message without topic, syncing all accounts") - for (account in accountRepository.getAll()) - syncWorkerManager.enqueueOneTimeAllAuthorities(account, fromPush = true) - } - } + runBlocking { + pushMessageHandler.get().processMessage(message, instance) } } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt index ad50c6a45f..5aebdce1e2 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt @@ -193,7 +193,7 @@ class DavCollectionRepository @Inject constructor( } } - fun getSyncableByTopic(topic: String) = dao.getSyncableByPushTopic(topic) + suspend fun getSyncableByTopic(topic: String) = dao.getSyncableByPushTopic(topic) fun get(id: Long) = dao.get(id) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt index 167b9f2947..8556d10b12 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt @@ -19,6 +19,7 @@ class DavServiceRepository @Inject constructor( // Read fun get(id: Long): Service? = dao.get(id) + suspend fun getAsync(id: Long): Service? = dao.getAsync(id) suspend fun getAll(): List = dao.getAll() diff --git a/app/src/main/kotlin/at/bitfire/davdroid/util/CoroutineDispatcherModule.kt b/app/src/main/kotlin/at/bitfire/davdroid/util/CoroutineDispatcherModule.kt new file mode 100644 index 0000000000..ead326b8e9 --- /dev/null +++ b/app/src/main/kotlin/at/bitfire/davdroid/util/CoroutineDispatcherModule.kt @@ -0,0 +1,43 @@ +/* + * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details. + */ + +package at.bitfire.davdroid.util + +import dagger.Module +import dagger.Provides +import dagger.hilt.InstallIn +import dagger.hilt.components.SingletonComponent +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers +import javax.inject.Qualifier + +@Retention(AnnotationRetention.RUNTIME) +@Qualifier +annotation class DefaultDispatcher + +@Retention(AnnotationRetention.RUNTIME) +@Qualifier +annotation class IoDispatcher + +@Retention(AnnotationRetention.RUNTIME) +@Qualifier +annotation class MainDispatcher + +@Module +@InstallIn(SingletonComponent::class) +class CoroutineDispatcherModule { + + @Provides + @DefaultDispatcher + fun defaultDispatcher(): CoroutineDispatcher = Dispatchers.Default + + @Provides + @IoDispatcher + fun ioDispatcher(): CoroutineDispatcher = Dispatchers.IO + + @Provides + @DefaultDispatcher + fun mainDispatcher(): CoroutineDispatcher = Dispatchers.Main + +} \ No newline at end of file diff --git a/app/src/test/kotlin/at/bitfire/davdroid/push/PushMessageParserTest.kt b/app/src/test/kotlin/at/bitfire/davdroid/push/PushMessageParserTest.kt deleted file mode 100644 index 22c72b11d8..0000000000 --- a/app/src/test/kotlin/at/bitfire/davdroid/push/PushMessageParserTest.kt +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details. - */ - -package at.bitfire.davdroid.push - -import org.junit.Assert.assertEquals -import org.junit.Assert.assertNull -import org.junit.Test -import java.util.logging.Logger - -class PushMessageParserTest { - - private val parse = PushMessageParser(logger = Logger.getGlobal()) - - @Test - fun testInvalidXml() { - assertNull(parse("Non-XML content")) - } - - @Test - fun testWithXmlDeclAndTopic() { - val topic = parse( - "" + - "" + - " O7M1nQ7cKkKTKsoS_j6Z3w" + - "" - ) - assertEquals("O7M1nQ7cKkKTKsoS_j6Z3w", topic) - } - -} \ No newline at end of file From 6ebe97ccd2807dcadcc45af346106ab9697869dc Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Thu, 24 Apr 2025 12:52:23 +0200 Subject: [PATCH 19/20] Add async support for push subscription updates --- .../CollectionListRefresherTest.kt | 6 +- .../at/bitfire/davdroid/db/CollectionDao.kt | 2 +- .../at/bitfire/davdroid/network/HttpClient.kt | 15 ++ .../davdroid/push/PushRegistrationManager.kt | 132 +++++++++--------- .../repository/DavCollectionRepository.kt | 2 +- .../bitfire/davdroid/ui/AppSettingsModel.kt | 10 +- 6 files changed, 92 insertions(+), 75 deletions(-) diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/CollectionListRefresherTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/CollectionListRefresherTest.kt index db3450bf81..86549602cd 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/CollectionListRefresherTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/CollectionListRefresherTest.kt @@ -19,7 +19,7 @@ import dagger.hilt.android.testing.HiltAndroidTest import io.mockk.every import io.mockk.impl.annotations.MockK import io.mockk.junit4.MockKRule -import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.runTest import okhttp3.mockwebserver.Dispatcher import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer @@ -122,7 +122,7 @@ class CollectionListRefresherTest { // refreshHomesetsAndTheirCollections @Test - fun refreshHomesetsAndTheirCollections_addsNewCollection() { + fun refreshHomesetsAndTheirCollections_addsNewCollection() = runTest { // save homeset in DB val homesetId = db.homeSetDao().insert( HomeSet(id=0, service.id, true, mockServer.url("$PATH_CARDDAV$SUBPATH_ADDRESSBOOK_HOMESET_PERSONAL")) @@ -143,7 +143,7 @@ class CollectionListRefresherTest { displayName = "My Contacts", description = "My Contacts Description" ), - runBlocking { db.collectionDao().getByService(service.id).first() } + db.collectionDao().getByService(service.id).first() ) } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt index 39032d82b8..a845341d91 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt @@ -97,7 +97,7 @@ interface CollectionDao { suspend fun updateForceReadOnly(id: Long, forceReadOnly: Boolean) @Query("UPDATE collection SET pushSubscription=:pushSubscription, pushSubscriptionExpires=:pushSubscriptionExpires, pushSubscriptionCreated=:updatedAt WHERE id=:id") - fun updatePushSubscription(id: Long, pushSubscription: String?, pushSubscriptionExpires: Long?, updatedAt: Long = System.currentTimeMillis()/1000) + suspend fun updatePushSubscription(id: Long, pushSubscription: String?, pushSubscriptionExpires: Long?, updatedAt: Long = System.currentTimeMillis()/1000) @Query("UPDATE collection SET sync=:sync WHERE id=:id") suspend fun updateSync(id: Long, sync: Boolean) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClient.kt b/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClient.kt index e675a0d3e8..364712c879 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClient.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClient.kt @@ -6,6 +6,7 @@ package at.bitfire.davdroid.network import android.accounts.Account import android.content.Context +import androidx.annotation.WorkerThread import at.bitfire.cert4android.CustomCertManager import at.bitfire.dav4jvm.BasicDigestAuthHandler import at.bitfire.dav4jvm.UrlUtils @@ -14,7 +15,10 @@ import at.bitfire.davdroid.settings.AccountSettings import at.bitfire.davdroid.settings.Settings import at.bitfire.davdroid.settings.SettingsManager import at.bitfire.davdroid.ui.ForegroundTracker +import at.bitfire.davdroid.util.IoDispatcher import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.withContext import net.openid.appauth.AuthState import net.openid.appauth.AuthorizationService import okhttp3.Authenticator @@ -64,6 +68,7 @@ class HttpClient( private val authorizationServiceProvider: Provider, @ApplicationContext private val context: Context, defaultLogger: Logger, + @IoDispatcher private val ioDispatcher: CoroutineDispatcher, private val keyManagerFactory: ClientCertKeyManager.Factory, private val settingsManager: SettingsManager ) { @@ -141,9 +146,12 @@ class HttpClient( /** * Takes authentication (basic/digest or OAuth and client certificate) from a given account. * + * **Must not be run on main thread, because it creates [AccountSettings]!** Use [fromAccountAsync] if possible. + * * @param account the account to take authentication from * @param onlyHost if set: only authenticate for this host name */ + @WorkerThread fun fromAccount(account: Account, onlyHost: String? = null): Builder { val accountSettings = accountSettingsFactory.create(account) authenticate( @@ -156,6 +164,13 @@ class HttpClient( return this } + /** + * Same as [fromAccount], but can be called on any thread. + */ + suspend fun fromAccountAsync(account: Account, onlyHost: String? = null): Builder = withContext(ioDispatcher) { + fromAccount(account, onlyHost) + } + // actual builder diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt index 4ce45ce6f9..a7560f0979 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt @@ -34,10 +34,10 @@ import at.bitfire.davdroid.util.IoDispatcher import dagger.Lazy import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.runInterruptible import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock -import kotlinx.coroutines.withContext import okhttp3.HttpUrl import okhttp3.HttpUrl.Companion.toHttpUrlOrNull import okhttp3.RequestBody.Companion.toRequestBody @@ -125,29 +125,27 @@ class PushRegistrationManager @Inject constructor( if (subscribeTo.isEmpty()) return - withContext(ioDispatcher) { // HttpClientBuilder.fromAccount() must not run in main thread and is not suspending - val account = accountRepository.get().fromName(service.accountName) - httpClientBuilder.get() - .fromAccount(account) - .build() - .use { httpClient -> - for (collection in subscribeTo) - try { - val expires = collection.pushSubscriptionExpires - // calculate next run time, but use the duplicate interval for safety (times are not exact) - val nextRun = Instant.now() + Duration.ofDays(2 * WORKER_INTERVAL_DAYS) - if (expires != null && expires >= nextRun.epochSecond) - logger.fine("Push subscription for ${collection.url} is still valid until ${collection.pushSubscriptionExpires}") - else { - // no existing subscription or expiring soon - logger.fine("Registering push subscription for ${collection.url}") - subscribe(httpClient, collection, endpoint) - } - } catch (e: Exception) { - logger.log(Level.WARNING, "Couldn't register subscription at CalDAV/CardDAV server", e) + val account = accountRepository.get().fromName(service.accountName) + httpClientBuilder.get() + .fromAccountAsync(account) + .build() + .use { httpClient -> + for (collection in subscribeTo) + try { + val expires = collection.pushSubscriptionExpires + // calculate next run time, but use the duplicate interval for safety (times are not exact) + val nextRun = Instant.now() + Duration.ofDays(2 * WORKER_INTERVAL_DAYS) + if (expires != null && expires >= nextRun.epochSecond) + logger.fine("Push subscription for ${collection.url} is still valid until ${collection.pushSubscriptionExpires}") + else { + // no existing subscription or expiring soon + logger.fine("Registering push subscription for ${collection.url}") + subscribe(httpClient, collection, endpoint) } - } - } + } catch (e: Exception) { + logger.log(Level.WARNING, "Couldn't register subscription at CalDAV/CardDAV server", e) + } + } } private suspend fun unsubscribeNotSyncable(service: Service) { @@ -155,19 +153,17 @@ class PushRegistrationManager @Inject constructor( if (unsubscribeFrom.isEmpty()) return - withContext(ioDispatcher) { // HttpClientBuilder.fromAccount() must not run in main thread and is not suspending - val account = accountRepository.get().fromName(service.accountName) - httpClientBuilder.get() - .fromAccount(account) - .build() - .use { httpClient -> - for (collection in unsubscribeFrom) - collection.pushSubscription?.toHttpUrlOrNull()?.let { url -> - logger.info("Unregistering push for ${collection.url}") - unsubscribe(httpClient, collection, url) - } - } - } + val account = accountRepository.get().fromName(service.accountName) + httpClientBuilder.get() + .fromAccountAsync(account) + .build() + .use { httpClient -> + for (collection in unsubscribeFrom) + collection.pushSubscription?.toHttpUrlOrNull()?.let { url -> + logger.info("Unregistering push for ${collection.url}") + unsubscribe(httpClient, collection, url) + } + } } /** @@ -181,19 +177,17 @@ class PushRegistrationManager @Inject constructor( if (unsubscribeFrom.isEmpty()) return - withContext(ioDispatcher) { // HttpClientBuilder.fromAccount() must not run in main thread and is not suspending - val account = accountRepository.get().fromName(service.accountName) - httpClientBuilder.get() - .fromAccount(account) - .build() - .use { httpClient -> - for (collection in unsubscribeFrom) - collection.pushSubscription?.toHttpUrlOrNull()?.let { url -> - logger.info("Unregistering push for ${collection.url}") - unsubscribe(httpClient, collection, url) - } - } - } + val account = accountRepository.get().fromName(service.accountName) + httpClientBuilder.get() + .fromAccountAsync(account) + .build() + .use { httpClient -> + for (collection in unsubscribeFrom) + collection.pushSubscription?.toHttpUrlOrNull()?.let { url -> + logger.info("Unregistering push for ${collection.url}") + unsubscribe(httpClient, collection, url) + } + } } @@ -237,7 +231,7 @@ class PushRegistrationManager @Inject constructor( } serializer.endDocument() - runInterruptible(ioDispatcher) { // network traffic + runInterruptible(ioDispatcher) { val xml = writer.toString().toRequestBody(DavResource.MIME_XML) DavCollection(httpClient.okHttpClient, collection.url).post(xml) { response -> if (response.isSuccessful) { @@ -246,11 +240,14 @@ class PushRegistrationManager @Inject constructor( val expires = response.header("Expires")?.let { expiresDate -> HttpUtils.parseDate(expiresDate) } ?: requestedExpiration - collectionRepository.updatePushSubscription( - id = collection.id, - subscriptionUrl = subscriptionUrl, - expires = expires?.epochSecond - ) + + runBlocking { + collectionRepository.updatePushSubscription( + id = collection.id, + subscriptionUrl = subscriptionUrl, + expires = expires?.epochSecond + ) + } } else logger.warning("Couldn't register push for ${collection.url}: $response") } @@ -258,22 +255,22 @@ class PushRegistrationManager @Inject constructor( } private suspend fun unsubscribe(httpClient: HttpClient, collection: Collection, url: HttpUrl) { - runInterruptible(ioDispatcher) { // network traffic - try { + try { + runInterruptible(ioDispatcher) { DavResource(httpClient.okHttpClient, url).delete { // deleted } - } catch (e: DavException) { - logger.log(Level.WARNING, "Couldn't unregister push for ${collection.url}", e) } - - // remove registration URL from DB in any case - collectionRepository.updatePushSubscription( - id = collection.id, - subscriptionUrl = null, - expires = null - ) + } catch (e: DavException) { + logger.log(Level.WARNING, "Couldn't unregister push for ${collection.url}", e) } + + // remove registration URL from DB in any case + collectionRepository.updatePushSubscription( + id = collection.id, + subscriptionUrl = null, + expires = null + ) } @@ -316,6 +313,9 @@ class PushRegistrationManager @Inject constructor( private const val WORKER_UNIQUE_NAME = "push-registration" const val WORKER_INTERVAL_DAYS = 1L + /** + * Mutex to synchronize (un)subscription. + */ val mutex = Mutex() } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt index 5aebdce1e2..603037cae5 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt @@ -274,7 +274,7 @@ class DavCollectionRepository @Inject constructor( notifyOnChangeListeners() } - fun updatePushSubscription(id: Long, subscriptionUrl: String?, expires: Long?) { + suspend fun updatePushSubscription(id: Long, subscriptionUrl: String?, expires: Long?) { dao.updatePushSubscription( id = id, pushSubscription = subscriptionUrl, diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt index f9c33e80fe..d04ea834c6 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt @@ -21,11 +21,12 @@ import at.bitfire.davdroid.settings.SettingsManager import at.bitfire.davdroid.sync.TasksAppManager import at.bitfire.davdroid.ui.intro.BatteryOptimizationsPageModel import at.bitfire.davdroid.ui.intro.OpenSourcePage +import at.bitfire.davdroid.util.IoDispatcher import at.bitfire.davdroid.util.PermissionUtils import at.bitfire.davdroid.util.broadcastReceiverFlow import dagger.hilt.android.lifecycle.HiltViewModel import dagger.hilt.android.qualifiers.ApplicationContext -import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.asStateFlow @@ -37,7 +38,8 @@ import javax.inject.Inject @HiltViewModel class AppSettingsModel @Inject constructor( - @ApplicationContext val context: Context, + @ApplicationContext private val context: Context, + @IoDispatcher private val ioDispatcher: CoroutineDispatcher, private val preferences: PreferenceRepository, private val pushRegistrationManager: PushRegistrationManager, private val settings: SettingsManager, @@ -160,7 +162,7 @@ class AppSettingsModel @Inject constructor( * @param pushDistributor The package name of the push distributor, _null_ to disable push. */ fun updatePushDistributor(pushDistributor: String?) { - viewModelScope.launch(Dispatchers.IO) { + viewModelScope.launch(ioDispatcher) { if (pushDistributor == null) { // Disable UnifiedPush if the distributor given is null UnifiedPush.removeDistributor(context) @@ -177,7 +179,7 @@ class AppSettingsModel @Inject constructor( init { - viewModelScope.launch(Dispatchers.IO) { + viewModelScope.launch(ioDispatcher) { loadPushDistributors() } } From 88512ce96d8d54841225f50262d17bb7b1a06189 Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Thu, 24 Apr 2025 13:02:16 +0200 Subject: [PATCH 20/20] Refactor unsubscribe logic into reusable method --- .../davdroid/push/PushRegistrationManager.kt | 58 ++++++++----------- 1 file changed, 25 insertions(+), 33 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt index a7560f0979..934ddbdf61 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt @@ -116,8 +116,11 @@ class PushRegistrationManager @Inject constructor( internal suspend fun processSubscription(serviceId: Long, endpoint: PushEndpoint) = mutex.withLock { val service = serviceRepository.getAsync(serviceId) ?: return + // subscribe to collections which are selected for synchronization subscribeSyncable(service, endpoint) - unsubscribeNotSyncable(service) + + // unsubscribe from collections which are not selected for synchronization + unsubscribeCollections(service, collectionRepository.getPushRegisteredAndNotSyncable(service.id)) } private suspend fun subscribeSyncable(service: Service, endpoint: PushEndpoint) { @@ -148,24 +151,6 @@ class PushRegistrationManager @Inject constructor( } } - private suspend fun unsubscribeNotSyncable(service: Service) { - val unsubscribeFrom = collectionRepository.getPushRegisteredAndNotSyncable(service.id) - if (unsubscribeFrom.isEmpty()) - return - - val account = accountRepository.get().fromName(service.accountName) - httpClientBuilder.get() - .fromAccountAsync(account) - .build() - .use { httpClient -> - for (collection in unsubscribeFrom) - collection.pushSubscription?.toHttpUrlOrNull()?.let { url -> - logger.info("Unregistering push for ${collection.url}") - unsubscribe(httpClient, collection, url) - } - } - } - /** * Called when no subscription is available (anymore) for the given service. * @@ -174,20 +159,7 @@ class PushRegistrationManager @Inject constructor( internal suspend fun removeSubscription(serviceId: Long) = mutex.withLock { val service = serviceRepository.getAsync(serviceId) ?: return val unsubscribeFrom = collectionRepository.getPushRegistered(service.id) - if (unsubscribeFrom.isEmpty()) - return - - val account = accountRepository.get().fromName(service.accountName) - httpClientBuilder.get() - .fromAccountAsync(account) - .build() - .use { httpClient -> - for (collection in unsubscribeFrom) - collection.pushSubscription?.toHttpUrlOrNull()?.let { url -> - logger.info("Unregistering push for ${collection.url}") - unsubscribe(httpClient, collection, url) - } - } + unsubscribeCollections(service, unsubscribeFrom) } @@ -254,6 +226,26 @@ class PushRegistrationManager @Inject constructor( } } + /** + * Unsubscribe from the given collections. + */ + private suspend fun unsubscribeCollections(service: Service, from: List) { + if (from.isEmpty()) + return + + val account = accountRepository.get().fromName(service.accountName) + httpClientBuilder.get() + .fromAccountAsync(account) + .build() + .use { httpClient -> + for (collection in from) + collection.pushSubscription?.toHttpUrlOrNull()?.let { url -> + logger.info("Unsubscribing Push from ${collection.url}") + unsubscribe(httpClient, collection, url) + } + } + } + private suspend fun unsubscribe(httpClient: HttpClient, collection: Collection, url: HttpUrl) { try { runInterruptible(ioDispatcher) {