Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Sync on collection update #1240

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions app/src/androidTest/kotlin/at/bitfire/davdroid/TestModules.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,24 @@ 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 at.bitfire.davdroid.sync.worker.SyncWorkerManager
import dagger.Module
import dagger.hilt.components.SingletonComponent
import dagger.hilt.testing.TestInstallIn
import dagger.multibindings.Multibinds

// remove SyncWorkerModule from Android tests
@Module
@TestInstallIn(
components = [SingletonComponent::class],
replaces = [SyncWorkerManager.SyncWorkerManagerModule::class]
)
abstract class TestSyncWorkerManagerModule {
// provides empty set of listeners
@Multibinds
abstract fun empty(): Set<DavCollectionRepository.OnChangeListener>
}

// remove PushRegistrationWorkerModule from Android tests
@Module
@TestInstallIn(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ class DavCollectionRepositoryTest {
@get:Rule
var hiltRule = HiltAndroidRule(this)

@Inject
lateinit var accountRepository: Lazy<AccountRepository>

@Inject
lateinit var accountSettingsFactory: AccountSettings.Factory

Expand Down Expand Up @@ -69,6 +72,7 @@ class DavCollectionRepositoryTest {
)
val testObserver = mockk<DavCollectionRepository.OnChangeListener>(relaxed = true)
val collectionRepository = DavCollectionRepository(
accountRepository,
accountSettingsFactory,
context,
db,
Expand All @@ -82,12 +86,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())
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -79,7 +80,7 @@ class PushRegistrationWorkerManager @Inject constructor(
val workerManager: PushRegistrationWorkerManager
): DavCollectionRepository.OnChangeListener {

override fun onCollectionsChanged() {
override fun onCollectionsChanged(account: Account?) {
workerManager.updatePeriodicWorker()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -161,6 +162,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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import javax.inject.Inject
* Implements an observer pattern that can be used to listen for changes of collections.
*/
class DavCollectionRepository @Inject constructor(
private val accountRepository: Lazy<AccountRepository>,
private val accountSettingsFactory: AccountSettings.Factory,
@ApplicationContext private val context: Context,
private val db: AppDatabase,
Expand Down Expand Up @@ -108,7 +109,7 @@ class DavCollectionRepository @Inject constructor(
)
dao.insertAsync(collection)

notifyOnChangeListeners()
notifyOnChangeListeners(account)
}

/**
Expand Down Expand Up @@ -168,7 +169,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. */
Expand Down Expand Up @@ -245,7 +246,7 @@ class DavCollectionRepository @Inject constructor(
*/
fun insertOrUpdateByUrl(collection: Collection) {
dao.insertOrUpdateByUrl(collection)
notifyOnChangeListeners()
notifyOnChangeListeners(getAccount(collection.id))
}

fun pageByServiceAndType(serviceId: Long, type: String) =
Expand All @@ -259,15 +260,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?) {
Expand All @@ -283,12 +284,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) {
HttpClient.Builder(context, accountSettingsFactory.create(account))
.setForeground(true)
Expand Down Expand Up @@ -419,9 +427,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)
}
}

Expand All @@ -431,7 +439,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import androidx.work.WorkManager
import androidx.work.WorkQuery
import androidx.work.WorkRequest
import at.bitfire.davdroid.push.PushNotificationManager
import at.bitfire.davdroid.repository.DavCollectionRepository
import at.bitfire.davdroid.sync.SyncDataType
import at.bitfire.davdroid.sync.TasksAppManager
import at.bitfire.davdroid.sync.worker.BaseSyncWorker.Companion.INPUT_ACCOUNT_NAME
Expand All @@ -36,8 +37,13 @@ import at.bitfire.davdroid.sync.worker.BaseSyncWorker.Companion.INPUT_UPLOAD
import at.bitfire.davdroid.sync.worker.BaseSyncWorker.Companion.InputResync
import at.bitfire.davdroid.sync.worker.BaseSyncWorker.Companion.NO_RESYNC
import at.bitfire.davdroid.sync.worker.BaseSyncWorker.Companion.commonTag
import dagger.Binds
import dagger.Lazy
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.flow.Flow
import kotlinx.coroutines.flow.map
import java.util.concurrent.TimeUnit
Expand Down Expand Up @@ -260,7 +266,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
Expand All @@ -285,4 +291,28 @@ class SyncWorkerManager @Inject constructor(
}
}

/**
* Listener that enqueues a push registration worker when the collection list changes.
*/
class CollectionsListener @Inject constructor(
private val workerManager: SyncWorkerManager
): DavCollectionRepository.OnChangeListener {

override fun onCollectionsChanged(account: Account?) {
account?.let { workerManager.enqueueOneTimeAllAuthorities(it) }
}

}

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

}
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ class AccountScreenModel @AssistedInject constructor(
}

fun setCollectionSync(id: Long, sync: Boolean) {
viewModelScope.launch {
viewModelScope.launch(Dispatchers.IO) {
collectionRepository.setSync(id, sync)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}
Expand Down
Loading