Skip to content

Commit

Permalink
[Push] Handle subscription expiration (#1131)
Browse files Browse the repository at this point in the history 
* Store subscription expiration in DB

* Regularly run PushRegistrationWorker, if needed

* Skip re-registering subscriptions that are not about to expire

* Add back-off for PushRegistrationWorkerManager

* Request expiration in 3 days

* Show expiration in UI, timestamps in seconds

* Fix tests
  • Loading branch information
@rfc2822
rfc2822 authored Nov 17, 2024
1 parent 32925dc commit 3a16b5c
Show file tree
Hide file tree
Showing 12 changed files with 878 additions and 113 deletions.
675 changes: 675 additions & 0 deletions app/schemas/at.bitfire.davdroid.db.AppDatabase/15.json
View file

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions app/src/androidTest/kotlin/at/bitfire/davdroid/TestModules.kt
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package at.bitfire.davdroid

import at.bitfire.davdroid.push.PushRegistrationWorker
import at.bitfire.davdroid.push.PushRegistrationWorkerManager
import at.bitfire.davdroid.repository.DavCollectionRepository
import at.bitfire.davdroid.startup.StartupPlugin
import at.bitfire.davdroid.startup.TasksAppWatcher
Expand All @@ -15,7 +15,7 @@ interface TestModules {
@Module
@TestInstallIn(
components = [SingletonComponent::class],
replaces = [PushRegistrationWorker.PushRegistrationWorkerModule::class]
replaces = [PushRegistrationWorkerManager.PushRegistrationWorkerModule::class]
)
abstract class TestPushRegistrationWorkerModule {
// provides empty set of listeners
Expand Down
13 changes: 12 additions & 1 deletion app/src/androidTest/kotlin/at/bitfire/davdroid/repository/DavCollectionRepositoryTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import at.bitfire.davdroid.db.AppDatabase
import at.bitfire.davdroid.db.Collection
import at.bitfire.davdroid.db.Service
import at.bitfire.davdroid.settings.AccountSettings
import dagger.Lazy
import dagger.hilt.android.qualifiers.ApplicationContext
import dagger.hilt.android.testing.HiltAndroidRule
import dagger.hilt.android.testing.HiltAndroidTest
Expand Down Expand Up @@ -63,7 +64,17 @@ class DavCollectionRepositoryTest {
)
)
val testObserver = mockk<DavCollectionRepository.OnChangeListener>(relaxed = true)
val collectionRepository = DavCollectionRepository(accountSettingsFactory, context, db, mutableSetOf(testObserver), serviceRepository)
val collectionRepository = DavCollectionRepository(
accountSettingsFactory,
context,
db,
object : Lazy<Set<DavCollectionRepository.OnChangeListener>> {
override fun get(): Set<DavCollectionRepository.OnChangeListener> {
return mutableSetOf(testObserver)
}
},
serviceRepository
)

assert(db.collectionDao().get(collectionId)?.forceReadOnly == false)
verify(exactly = 0) {
Expand Down
27 changes: 3 additions & 24 deletions app/src/main/kotlin/at/bitfire/davdroid/db/AppDatabase.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,13 @@ import javax.inject.Singleton
SyncStats::class,
WebDavDocument::class,
WebDavMount::class
], exportSchema = true, version = 14, autoMigrations = [
], exportSchema = true, version = 15, autoMigrations = [
AutoMigration(from = 9, to = 10),
AutoMigration(from = 10, to = 11),
AutoMigration(from = 11, to = 12, spec = AppDatabase.AutoMigration11_12::class),
AutoMigration(from = 12, to = 13),
AutoMigration(from = 13, to = 14)
AutoMigration(from = 13, to = 14),
AutoMigration(from = 14, to = 15)
])
@TypeConverters(Converters::class)
abstract class AppDatabase: RoomDatabase() {
Expand Down Expand Up @@ -218,28 +219,6 @@ abstract class AppDatabase: RoomDatabase() {
// We don't have access to the context in a Room migration now, so
// we will just drop those settings from old DAVx5 versions.
Logger.getGlobal().warning("Dropping settings distrustSystemCerts and overrideProxy*")

/*val edit = PreferenceManager.getDefaultSharedPreferences(context).edit()
try {
db.query("settings", arrayOf("setting", "value"), null, null, null, null, null).use { cursor ->
while (cursor.moveToNext()) {
when (cursor.getString(0)) {
"distrustSystemCerts" -> edit.putBoolean(App.DISTRUST_SYSTEM_CERTIFICATES, cursor.getInt(1) != 0)
"overrideProxy" -> edit.putBoolean(App.OVERRIDE_PROXY, cursor.getInt(1) != 0)
"overrideProxyHost" -> edit.putString(App.OVERRIDE_PROXY_HOST, cursor.getString(1))
"overrideProxyPort" -> edit.putInt(App.OVERRIDE_PROXY_PORT, cursor.getInt(1))
StartupDialogFragment.HINT_GOOGLE_PLAY_ACCOUNTS_REMOVED ->
edit.putBoolean(StartupDialogFragment.HINT_GOOGLE_PLAY_ACCOUNTS_REMOVED, cursor.getInt(1) != 0)
StartupDialogFragment.HINT_OPENTASKS_NOT_INSTALLED ->
edit.putBoolean(StartupDialogFragment.HINT_OPENTASKS_NOT_INSTALLED, cursor.getInt(1) != 0)
}
}
}
db.execSQL("DROP TABLE settings")
} finally {
edit.apply()
}*/
}
},

Expand Down
5 changes: 4 additions & 1 deletion app/src/main/kotlin/at/bitfire/davdroid/db/Collection.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,10 @@ data class Collection(
/** WebDAV-Push subscription URL */
var pushSubscription: String? = null,

/** when the [pushSubscription] was created/updated (used to determine whether we need to re-subscribe) */
/** when the [pushSubscription] expires (timestamp, used to determine whether we need to re-subscribe) */
var pushSubscriptionExpires: Long? = null,

/** when the [pushSubscription] was created/updated (timestamp) */
var pushSubscriptionCreated: Long? = null

) {
Expand Down
7 changes: 5 additions & 2 deletions app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ interface CollectionDao {
@Query("SELECT COUNT(*) FROM collection WHERE serviceId=:serviceId AND type=:type")
suspend fun anyOfType(serviceId: Long, type: String): Boolean

@Query("SELECT COUNT(*) FROM collection WHERE supportsWebPush AND pushTopic IS NOT NULL")
suspend fun anyPushCapable(): Boolean

/**
* Returns collections which
* - support VEVENT and/or VTODO (= supported calendar collections), or
Expand Down Expand Up @@ -87,8 +90,8 @@ interface CollectionDao {
@Query("UPDATE collection SET forceReadOnly=:forceReadOnly WHERE id=:id")
suspend fun updateForceReadOnly(id: Long, forceReadOnly: Boolean)

@Query("UPDATE collection SET pushSubscription=:pushSubscription, pushSubscriptionCreated=:updatedAt WHERE id=:id")
fun updatePushSubscription(id: Long, pushSubscription: String?, updatedAt: Long = System.currentTimeMillis())
@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)

@Query("UPDATE collection SET sync=:sync WHERE id=:id")
suspend fun updateSync(id: Long, sync: Boolean)
Expand Down
107 changes: 41 additions & 66 deletions app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationWorker.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,11 @@ package at.bitfire.davdroid.push
import android.accounts.Account
import android.content.Context
import androidx.hilt.work.HiltWorker
import androidx.work.Constraints
import androidx.work.CoroutineWorker
import androidx.work.ExistingWorkPolicy
import androidx.work.NetworkType
import androidx.work.OneTimeWorkRequestBuilder
import androidx.work.WorkManager
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
Expand All @@ -28,31 +24,23 @@ import at.bitfire.davdroid.repository.DavCollectionRepository
import at.bitfire.davdroid.repository.DavServiceRepository
import at.bitfire.davdroid.repository.PreferenceRepository
import at.bitfire.davdroid.settings.AccountSettings
import dagger.Binds
import dagger.Module
import dagger.assisted.Assisted
import dagger.assisted.AssistedInject
import dagger.hilt.InstallIn
import dagger.hilt.android.qualifiers.ApplicationContext
import dagger.hilt.components.SingletonComponent
import dagger.multibindings.IntoSet
import kotlinx.coroutines.runInterruptible
import okhttp3.HttpUrl
import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
import okhttp3.RequestBody.Companion.toRequestBody
import java.io.IOException
import java.io.StringWriter
import java.util.concurrent.TimeUnit
import java.time.Duration
import java.time.Instant
import java.util.logging.Level
import java.util.logging.Logger
import javax.inject.Inject

/**
* 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).
*
* TODO Should run periodically, too (to refresh registrations that are about to expire).
* Not required for a first demonstration version.
*/
@Suppress("unused")
@HiltWorker
Expand All @@ -66,34 +54,15 @@ class PushRegistrationWorker @AssistedInject constructor(
private val serviceRepository: DavServiceRepository
) : CoroutineWorker(context, workerParameters) {

companion object {

private const val UNIQUE_WORK_NAME = "push-registration"

/**
* Enqueues a push registration worker with a minimum delay of 5 seconds.
*/
fun enqueue(context: Context) {
val constraints = Constraints.Builder()
.setRequiredNetworkType(NetworkType.CONNECTED) // require a network connection
.build()
val workRequest = OneTimeWorkRequestBuilder<PushRegistrationWorker>()
.setInitialDelay(5, TimeUnit.SECONDS)
.setConstraints(constraints)
.build()
Logger.getGlobal().info("Enqueueing push registration worker")
WorkManager.getInstance(context)
.enqueueUniqueWork(UNIQUE_WORK_NAME, ExistingWorkPolicy.REPLACE, workRequest)
}

}


override suspend fun doWork(): Result {
logger.info("Running push registration worker")

registerSyncable()
unregisterNotSyncable()
try {
registerSyncable()
unregisterNotSyncable()
} catch (_: IOException) {
return Result.retry() // retry on I/O errors
}

return Result.success()
}
Expand All @@ -108,27 +77,41 @@ class PushRegistrationWorker @AssistedInject constructor(
.use { client ->
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(Property.Name(NS_WEBDAV_PUSH, "push-register")) {
serializer.insertTag(Property.Name(NS_WEBDAV_PUSH, "subscription")) {
// subscription URL
serializer.insertTag(Property.Name(NS_WEBDAV_PUSH, "web-push-subscription")) {
serializer.insertTag(Property.Name(NS_WEBDAV_PUSH, "push-resource")) {
text(endpoint)
}
}
// requested expiration
serializer.insertTag(Property.Name(NS_WEBDAV_PUSH, "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) {
response.header("Location")?.let { subscriptionUrl ->
collectionRepository.updatePushSubscription(collection.id, subscriptionUrl)
}
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")
}
Expand All @@ -142,6 +125,15 @@ class PushRegistrationWorker @AssistedInject constructor(
// register push subscription for syncable collections
if (endpoint != null)
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))
Expand Down Expand Up @@ -176,7 +168,11 @@ class PushRegistrationWorker @AssistedInject constructor(
}

// remove registration URL from DB in any case
collectionRepository.updatePushSubscription(collection.id, null)
collectionRepository.updatePushSubscription(
id = collection.id,
subscriptionUrl = null,
expires = null
)
}
}
}
Expand All @@ -193,25 +189,4 @@ class PushRegistrationWorker @AssistedInject constructor(
}
}


/**
* Listener that enqueues a push registration worker when the collection list changes.
*/
class CollectionsListener @Inject constructor(
@ApplicationContext val context: Context
): DavCollectionRepository.OnChangeListener {
override fun onCollectionsChanged() = enqueue(context)
}

/**
* Hilt module that registers [CollectionsListener] in [DavCollectionRepository].
*/
@Module
@InstallIn(SingletonComponent::class)
interface PushRegistrationWorkerModule {
@Binds
@IntoSet
fun listener(impl: CollectionsListener): DavCollectionRepository.OnChangeListener
}

}
Loading

0 comments on commit 3a16b5c

Please sign in to comment.