Sync on collection update

app/src/androidTest/kotlin/at/bitfire/davdroid/TestModules.kt

@@ -6,13 +6,26 @@ package at.bitfire.davdroid
 
 import at.bitfire.davdroid.push.PushRegistrationWorkerManager
 import at.bitfire.davdroid.repository.DavCollectionRepository
+import at.bitfire.davdroid.repository.SyncOnCollectionsChangeListener.SyncOnCollectionsChangeListenerModule
 import at.bitfire.davdroid.startup.StartupPlugin
 import at.bitfire.davdroid.startup.TasksAppWatcher
 import dagger.Module
 import dagger.hilt.components.SingletonComponent
 import dagger.hilt.testing.TestInstallIn
 import dagger.multibindings.Multibinds
 
+// remove SyncOnCollectionsChangeListenerModule from Android tests
+@Module
+@TestInstallIn(
+    components = [SingletonComponent::class],
+    replaces = [SyncOnCollectionsChangeListenerModule::class]
+)
+abstract class TestSyncOnCollectionsChangeListenerModule {
+    // provides empty set of listeners
+    @Multibinds
+    abstract fun empty(): Set<DavCollectionRepository.OnChangeListener>
+}
+
 // remove PushRegistrationWorkerModule from Android tests
 @Module
 @TestInstallIn(

app/src/androidTest/kotlin/at/bitfire/davdroid/repository/DavCollectionRepositoryTest.kt

@@ -76,12 +76,12 @@ class DavCollectionRepositoryTest {
 
         assert(db.collectionDao().get(collectionId)?.forceReadOnly == false)
         verify(exactly = 0) {
-            testObserver.onCollectionsChanged()
+            testObserver.onCollectionsChanged(any())
         }
         collectionRepository.setForceReadOnly(collectionId, true)
         assert(db.collectionDao().get(collectionId)?.forceReadOnly == true)
         verify(exactly = 1) {
-            testObserver.onCollectionsChanged()
+            testObserver.onCollectionsChanged(any())
         }
     }
 

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

@@ -4,6 +4,7 @@
 
 package at.bitfire.davdroid.push
 
+import android.accounts.Account
 import android.content.Context
 import androidx.work.BackoffPolicy
 import androidx.work.Constraints
@@ -79,7 +80,7 @@ class PushRegistrationWorkerManager @Inject constructor(
         val workerManager: PushRegistrationWorkerManager
     ): DavCollectionRepository.OnChangeListener {
 
-        override fun onCollectionsChanged() {
+        override fun onCollectionsChanged(account: Account?) {
             workerManager.updatePeriodicWorker()
         }
 

app/src/main/kotlin/at/bitfire/davdroid/repository/AccountRepository.kt

@@ -10,6 +10,7 @@ import android.accounts.OnAccountsUpdateListener
 import android.content.Context
 import at.bitfire.davdroid.InvalidAccountException
 import at.bitfire.davdroid.R
+import at.bitfire.davdroid.db.Collection
 import at.bitfire.davdroid.db.Credentials
 import at.bitfire.davdroid.db.HomeSet
 import at.bitfire.davdroid.db.Service
@@ -162,6 +163,13 @@ class AccountRepository @Inject constructor(
         }
     }
 
+    /**
+     * Returns the account for a given collection.
+     */
+    fun getByCollection(collection: Collection) = serviceRepository.get(collection.serviceId)?.let {
+        fromName(it.accountName)
+    }
+
     /**
      * Renames an account.
      *

app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt

@@ -58,7 +58,7 @@ import javax.inject.Provider
  * Implements an observer pattern that can be used to listen for changes of collections.
  */
 class DavCollectionRepository @Inject constructor(
-    @ApplicationContext private val context: Context,
+    private val accountRepository: Lazy<AccountRepository>, @ApplicationContext private val context: Context,
     private val db: AppDatabase,
     defaultListeners: Lazy<Set<@JvmSuppressWildcards OnChangeListener>>,
     private val httpClientBuilder: Provider<HttpClient.Builder>,
@@ -112,7 +112,7 @@ class DavCollectionRepository @Inject constructor(
         )
         dao.insertAsync(collection)
 
-        notifyOnChangeListeners()
+        notifyOnChangeListeners(account)
     }
 
     /**
@@ -172,7 +172,7 @@ class DavCollectionRepository @Inject constructor(
         // Some servers are known to change the supported components (VEVENT, …) after creation.
         RefreshCollectionsWorker.enqueue(context, homeSet.serviceId)
 
-        notifyOnChangeListeners()
+        notifyOnChangeListeners(account)
     }
 
     /** Deletes the given collection from the server and the database. */
@@ -249,7 +249,7 @@ class DavCollectionRepository @Inject constructor(
      */
     fun insertOrUpdateByUrl(collection: Collection) {
         dao.insertOrUpdateByUrl(collection)
-        notifyOnChangeListeners()
+        notifyOnChangeListeners(getAccount(collection.id))
     }
 
     fun pageByServiceAndType(serviceId: Long, @CollectionType type: String) =
@@ -263,15 +263,15 @@ class DavCollectionRepository @Inject constructor(
      */
     suspend fun setForceReadOnly(id: Long, forceReadOnly: Boolean) {
         dao.updateForceReadOnly(id, forceReadOnly)
-        notifyOnChangeListeners()
+        notifyOnChangeListeners(getAccount(id))
     }
 
     /**
      * Whether or not the local collection should be synced with the server
      */
     suspend fun setSync(id: Long, forceReadOnly: Boolean) {
         dao.updateSync(id, forceReadOnly)
-        notifyOnChangeListeners()
+        notifyOnChangeListeners(getAccount(id))
     }
 
     fun updatePushSubscription(id: Long, subscriptionUrl: String?, expires: Long?) {
@@ -287,12 +287,19 @@ class DavCollectionRepository @Inject constructor(
      */
     fun delete(collection: Collection) {
         dao.delete(collection)
-        notifyOnChangeListeners()
+        notifyOnChangeListeners(getAccount(collection.id))
     }
 
 
     // helpers
 
+    /**
+     * Returns the account the given collection belongs to.
+     */
+    fun getAccount(collectionId: Long) = dao.get(collectionId)?.let {
+        accountRepository.get().getByCollection(it)
+    }
+
     private suspend fun createOnServer(account: Account, url: HttpUrl, method: String, xmlBody: String) {
         httpClientBuilder.get()
             .fromAccount(account)
@@ -431,9 +438,9 @@ class DavCollectionRepository @Inject constructor(
     /**
      * Notifies registered listeners about changes in the collections.
      */
-    private fun notifyOnChangeListeners() = synchronized(listeners) {
+    private fun notifyOnChangeListeners(account: Account?) = synchronized(listeners) {
         listeners.forEach { listener ->
-            listener.onCollectionsChanged()
+            listener.onCollectionsChanged(account)
         }
     }
 
@@ -443,7 +450,7 @@ class DavCollectionRepository @Inject constructor(
          * of the data-modifying method. For instance, if [delete] is called, [onCollectionsChanged]
          * will be called in the context/thread that called [delete].
          */
-        fun onCollectionsChanged()
+        fun onCollectionsChanged(account: Account?)
     }
 
     @Module

app/src/main/kotlin/at/bitfire/davdroid/repository/SyncOnCollectionsChangeListener.kt

@@ -0,0 +1,58 @@
+/*
+ * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
+ */
+
+package at.bitfire.davdroid.repository
+
+import android.accounts.Account
+import at.bitfire.davdroid.sync.worker.SyncWorkerManager
+import dagger.Binds
+import dagger.Module
+import dagger.hilt.InstallIn
+import dagger.hilt.components.SingletonComponent
+import dagger.multibindings.IntoSet
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.Job
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.launch
+import java.util.logging.Logger
+import javax.inject.Inject
+
+/**
+ * Enqueues a sync worker after a short delay when the collection list changes.
+ */
+class SyncOnCollectionsChangeListener @Inject constructor(
+    private val workerManager: SyncWorkerManager,
+    private val logger: Logger
+): DavCollectionRepository.OnChangeListener {
+
+    var delayedOneTimeSyncWorkerJob: Job? = null
+
+    override fun onCollectionsChanged(account: Account?) {
+        account?.let {
+            // Start sync after a short delay to avoid multiple syncs in a short time when multiple
+            // collections change quickly, e.g. at collection refresh or users first time setup.
+            delayedOneTimeSyncWorkerJob?.cancel()
+            delayedOneTimeSyncWorkerJob = CoroutineScope(Dispatchers.IO).launch {
+                delay(7000)
+                logger.info("Collections changed, scheduling sync")
+                workerManager.enqueueOneTimeAllAuthorities(it)
+            }
+        }
+    }
+
+
+
+    /**
+     * Hilt module that registers [SyncOnCollectionsChangeListener] in [DavCollectionRepository].
+     */
+    @Module
+    @InstallIn(SingletonComponent::class)
+    interface SyncOnCollectionsChangeListenerModule {
+        @Binds
+        @IntoSet
+        fun listener(impl: SyncOnCollectionsChangeListener): DavCollectionRepository.OnChangeListener
+    }
+
+}

app/src/main/kotlin/at/bitfire/davdroid/sync/worker/SyncWorkerManager.kt

@@ -143,7 +143,7 @@ class SyncWorkerManager @Inject constructor(
         }
         WorkManager.getInstance(context).enqueueUniqueWork(
             name,
-            /* If sync is already running, just continue.
+            /* If sync is already running, continue that sync and don't append a new one.
             Existing retried work will not be replaced (for instance when
             PeriodicSyncWorker enqueues another scheduled sync). */
             ExistingWorkPolicy.KEEP,
@@ -260,7 +260,7 @@ class SyncWorkerManager @Inject constructor(
      *
      * @param workStates   list of states of workers to match
      * @param account      the account which the workers belong to
-     * @param authorities  type of sync work, ie [CalendarContract.AUTHORITY]
+     * @param dataTypes    type of sync work, ie [CalendarContract.AUTHORITY]
      * @param whichTag     function to generate tag that should be observed for given account and authority
      *
      * @return flow that emits `true` if at least one worker with matching query was found; `false` otherwise

app/src/main/kotlin/at/bitfire/davdroid/ui/account/AccountScreenModel.kt

@@ -183,7 +183,7 @@ class AccountScreenModel @AssistedInject constructor(
     }
 
     fun setCollectionSync(id: Long, sync: Boolean) {
-        viewModelScope.launch {
+        viewModelScope.launch(Dispatchers.IO) {
             collectionRepository.setSync(id, sync)
         }
     }

app/src/main/kotlin/at/bitfire/davdroid/ui/account/CollectionScreenModel.kt

@@ -21,6 +21,7 @@ import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedInject
 import dagger.hilt.android.lifecycle.HiltViewModel
 import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.SharingStarted
@@ -124,13 +125,13 @@ class CollectionScreenModel @AssistedInject constructor(
     }
 
     fun setForceReadOnly(forceReadOnly: Boolean) {
-        viewModelScope.launch {
+        viewModelScope.launch(Dispatchers.IO) {
             collectionRepository.setForceReadOnly(collectionId, forceReadOnly)
         }
     }
 
     fun setSync(sync: Boolean) {
-        viewModelScope.launch {
+        viewModelScope.launch(Dispatchers.IO) {
             collectionRepository.setSync(collectionId, sync)
         }
     }