Observable collections repository (#829)

* Call collection updates and setters only from repository

* Make collection repository changes observable

* Add kdoc

* Add basic test

* Extract RefreshCollectionsWorker; move some HomeSetDao calls to DavHomeSetRepository

* Replace more HomeSetDao calls

* Remove duplicate copyright notice

* Drop weak reference for observers

* Rename method

* Remove test service after run

* Verify notifying works with mockk

* Rename test

* Use construction injection

* Remove unused SettingsManager

* Remove obsolete mockk rule

* Use runBlocking instead of runTest

* Change to observer linkedList to mutableSetOf, remove synchronized calls

* Change to hilt multibinding

* Remove some unnecessary lines; allow empty set by Hilt

* CollectionListRefresher: delete collections using repository

* deleteRemote: call callback too; adapt KDoc

---------

Co-authored-by: Ricki Hirner <hirner@bitfire.at>
This commit is contained in:
Sunik Kupfer 2024-06-12 12:43:56 +02:00 committed by GitHub
parent 0bdeffe70d
commit 64563bbd3a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 187 additions and 39 deletions

View file

@ -0,0 +1,76 @@
package at.bitfire.davdroid.repository
import androidx.test.platform.app.InstrumentationRegistry
import at.bitfire.davdroid.db.AppDatabase
import at.bitfire.davdroid.db.Collection
import at.bitfire.davdroid.db.Service
import dagger.hilt.android.testing.HiltAndroidRule
import dagger.hilt.android.testing.HiltAndroidTest
import io.mockk.mockk
import io.mockk.verify
import kotlinx.coroutines.runBlocking
import okhttp3.HttpUrl.Companion.toHttpUrl
import org.junit.After
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import javax.inject.Inject
@HiltAndroidTest
class DavCollectionRepositoryTest {
@get:Rule
var hiltRule = HiltAndroidRule(this)
@Inject
lateinit var db: AppDatabase
val context = InstrumentationRegistry.getInstrumentation().targetContext
var service: Service? = null
@Before
fun setUp() {
hiltRule.inject()
service = createTestService(Service.TYPE_CARDDAV)!!
}
@After
fun cleanUp() {
db.close()
db.serviceDao().deleteAll()
}
@Test
fun testOnChangeListener_setForceReadOnly() = runBlocking {
val collectionId = db.collectionDao().insertOrUpdateByUrl(
Collection(
serviceId = service!!.id,
type = Collection.TYPE_ADDRESSBOOK,
url = "https://example.com".toHttpUrl(),
forceReadOnly = false,
)
)
val testObserver = mockk<DavCollectionRepository.OnChangeListener>(relaxed = true)
val collectionRepository = DavCollectionRepository(context, mutableSetOf(testObserver), db)
assert(db.collectionDao().get(collectionId)?.forceReadOnly == false)
verify(exactly = 0) {
testObserver.onCollectionsChanged()
}
collectionRepository.setForceReadOnly(collectionId, true)
assert(db.collectionDao().get(collectionId)?.forceReadOnly == true)
verify(exactly = 1) {
testObserver.onCollectionsChanged()
}
}
// Test helpers and dependencies
private fun createTestService(serviceType: String) : Service? {
val service = Service(id=0, accountName="test", type=serviceType, principal = null)
val serviceId = db.serviceDao().insertOrReplace(service)
return db.serviceDao().get(serviceId)
}
}

View file

@ -98,7 +98,7 @@ class CollectionListRefresherTest {
db.close()
}
@Test
fun testDiscoverHomesets() {
val service = createTestService(Service.TYPE_CARDDAV)!!

View file

@ -94,23 +94,6 @@ interface CollectionDao {
localCollection.id
} ?: insert(collection)
/**
* Inserts or updates the collection. On update it will not update flag values ([Collection.sync],
* [Collection.forceReadOnly]), but use the values of the already existing collection.
*
* @param newCollection Collection to be inserted or updated
*/
fun insertOrUpdateByUrlAndRememberFlags(newCollection: Collection) {
// remember locally set flags
getByServiceAndUrl(newCollection.serviceId, newCollection.url.toString())?.let { oldCollection ->
newCollection.sync = oldCollection.sync
newCollection.forceReadOnly = oldCollection.forceReadOnly
}
// commit to database
insertOrUpdateByUrl(newCollection)
}
@Delete
fun delete(collection: Collection)

View file

@ -16,7 +16,6 @@ import android.provider.ContactsContract
import androidx.core.content.ContextCompat
import at.bitfire.davdroid.InvalidAccountException
import at.bitfire.davdroid.R
import at.bitfire.davdroid.db.AppDatabase
import at.bitfire.davdroid.db.Credentials
import at.bitfire.davdroid.db.HomeSet
import at.bitfire.davdroid.db.Service
@ -49,10 +48,10 @@ import javax.inject.Inject
*/
class AccountRepository @Inject constructor(
val context: Application,
private val db: AppDatabase,
private val homeSetRepository: DavHomeSetRepository,
private val settingsManager: SettingsManager,
private val serviceRepository: DavServiceRepository
private val serviceRepository: DavServiceRepository,
private val collectionRepository: DavCollectionRepository
) {
private val accountType = context.getString(R.string.account_type)
@ -299,10 +298,9 @@ class AccountRepository @Inject constructor(
homeSetRepository.insertOrUpdateByUrl(HomeSet(0, serviceId, true, homeSet))
// insert collections
val collectionDao = db.collectionDao()
for (collection in info.collections.values) {
collection.serviceId = serviceId
collectionDao.insertOrUpdateByUrl(collection)
collectionRepository.insertOrUpdateByUrl(collection)
}
return serviceId

View file

@ -5,7 +5,7 @@
package at.bitfire.davdroid.repository
import android.accounts.Account
import android.app.Application
import android.content.Context
import at.bitfire.dav4jvm.DavResource
import at.bitfire.dav4jvm.XmlUtils
import at.bitfire.dav4jvm.XmlUtils.insertTag
@ -28,26 +28,40 @@ import at.bitfire.davdroid.servicedetection.RefreshCollectionsWorker
import at.bitfire.davdroid.settings.AccountSettings
import at.bitfire.davdroid.util.DavUtils
import at.bitfire.ical4android.util.DateUtils
import dagger.Module
import dagger.hilt.InstallIn
import dagger.hilt.android.qualifiers.ApplicationContext
import dagger.hilt.components.SingletonComponent
import dagger.multibindings.Multibinds
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.runInterruptible
import kotlinx.coroutines.withContext
import net.fortuna.ical4j.model.Component
import okhttp3.HttpUrl
import java.io.StringWriter
import java.util.Collections
import java.util.UUID
import javax.inject.Inject
/**
* Repository for managing collections.
*
* Implements an observer pattern that can be used to listen for changes of collections.
*/
class DavCollectionRepository @Inject constructor(
val context: Application,
@ApplicationContext val context: Context,
defaultListeners: Set<@JvmSuppressWildcards OnChangeListener>,
db: AppDatabase
) {
private val listeners = Collections.synchronizedSet(defaultListeners.toMutableSet())
private val serviceDao = db.serviceDao()
private val dao = db.collectionDao()
suspend fun anyWebcal(serviceId: Long) =
dao.anyOfType(serviceId, Collection.TYPE_WEBCAL)
/**
* Creates address book collection on server and locally
*/
suspend fun createAddressBook(
account: Account,
homeSet: HomeSet,
@ -77,13 +91,18 @@ class DavCollectionRepository @Inject constructor(
serviceId = homeSet.serviceId,
homeSetId = homeSet.id,
url = url,
type = Collection.TYPE_ADDRESSBOOK, //if (addressBook) Collection.TYPE_ADDRESSBOOK else Collection.TYPE_CALENDAR,
type = Collection.TYPE_ADDRESSBOOK,
displayName = displayName,
description = description
)
dao.insertAsync(collection)
notifyOnChangeListeners()
}
/**
* Create calendar collection on server and locally
*/
suspend fun createCalendar(
account: Account,
homeSet: HomeSet,
@ -137,10 +156,12 @@ class DavCollectionRepository @Inject constructor(
// Trigger service detection (because the collection may actually have other properties than the ones we have inserted).
// Some servers are known to change the supported components (VEVENT, …) after creation.
RefreshCollectionsWorker.enqueue(context, homeSet.serviceId)
notifyOnChangeListeners()
}
/** Deletes the given collection from the server and the database. */
suspend fun delete(collection: Collection) {
suspend fun deleteRemote(collection: Collection) {
val service = serviceDao.get(collection.serviceId) ?: throw IllegalArgumentException("Service not found")
val account = Account(service.accountName, context.getString(R.string.account_type))
@ -150,8 +171,8 @@ class DavCollectionRepository @Inject constructor(
withContext(Dispatchers.IO) {
runInterruptible {
DavResource(httpClient.okHttpClient, collection.url).delete() {
// success, otherwise an exception would have been thrown
dao.delete(collection)
// success, otherwise an exception would have been thrown → delete locally, too
delete(collection)
}
}
}
@ -160,14 +181,54 @@ class DavCollectionRepository @Inject constructor(
fun getFlow(id: Long) = dao.getFlow(id)
/**
* Sets the flag for whether read-only should be enforced on the local collection
*/
suspend fun setForceReadOnly(id: Long, forceReadOnly: Boolean) {
dao.updateForceReadOnly(id, forceReadOnly)
notifyOnChangeListeners()
}
/**
* Whether or not the local collection should be synced with the server
*/
suspend fun setSync(id: Long, forceReadOnly: Boolean) {
dao.updateSync(id, forceReadOnly)
notifyOnChangeListeners()
}
/**
* Inserts or updates the collection. On update it will not update flag values ([Collection.sync],
* [Collection.forceReadOnly]), but use the values of the already existing collection.
*
* @param newCollection Collection to be inserted or updated
*/
fun insertOrUpdateByUrlAndRememberFlags(newCollection: Collection) {
// remember locally set flags
dao.getByServiceAndUrl(newCollection.serviceId, newCollection.url.toString())?.let { oldCollection ->
newCollection.sync = oldCollection.sync
newCollection.forceReadOnly = oldCollection.forceReadOnly
}
// commit to database
insertOrUpdateByUrl(newCollection)
}
/**
* Creates or updates the existing collection if it exists (URL)
*/
fun insertOrUpdateByUrl(collection: Collection) {
dao.insertOrUpdateByUrl(collection)
notifyOnChangeListeners()
}
/**
* Deletes the collection locally
*/
fun delete(collection: Collection) {
dao.delete(collection)
notifyOnChangeListeners()
}
// helpers
@ -288,4 +349,32 @@ class DavCollectionRepository @Inject constructor(
private fun getVTimeZone(tzId: String): String? =
DateUtils.ical4jTimeZone(tzId)?.toString()
/*** OBSERVERS ***/
/**
* Notifies registered listeners about changes in the collections.
*/
private fun notifyOnChangeListeners() = synchronized(listeners) {
listeners.forEach { listener ->
listener.onCollectionsChanged()
}
}
fun interface OnChangeListener {
/**
* Will be called when collections have changed. Will run in the coroutine context/thread
* of the data-modifying method. For instance, if [delete] is called, [onCollectionsChanged]
* will be called in the context/thread that called [delete].
*/
fun onCollectionsChanged()
}
@Module
@InstallIn(SingletonComponent::class)
abstract class DavCollectionRepositoryModule {
@Multibinds abstract fun defaultOnChangeListeners(): Set<OnChangeListener>
}
}

View file

@ -25,6 +25,7 @@ import at.bitfire.davdroid.db.HomeSet
import at.bitfire.davdroid.db.Principal
import at.bitfire.davdroid.db.Service
import at.bitfire.davdroid.log.Logger
import at.bitfire.davdroid.repository.DavCollectionRepository
import at.bitfire.davdroid.repository.DavHomeSetRepository
import at.bitfire.davdroid.settings.Settings
import at.bitfire.davdroid.settings.SettingsManager
@ -44,6 +45,7 @@ class CollectionListRefresher @AssistedInject constructor(
@Assisted val httpClient: OkHttpClient,
private val db: AppDatabase,
private val homeSetRepository: DavHomeSetRepository,
private val collectionRepository: DavCollectionRepository,
private val settings: SettingsManager
) {
@ -215,7 +217,7 @@ class CollectionListRefresher @AssistedInject constructor(
// save or update collection if usable (ignore it otherwise)
if (isUsableCollection(collection))
db.collectionDao().insertOrUpdateByUrlAndRememberFlags(collection)
collectionRepository.insertOrUpdateByUrlAndRememberFlags(collection)
// Remove this collection from queue - because it was found in the home set
localHomesetCollections.remove(collection.url)
@ -229,7 +231,7 @@ class CollectionListRefresher @AssistedInject constructor(
// Mark leftover (not rediscovered) collections from queue as homeless (remove association)
for ((_, homelessCollection) in localHomesetCollections) {
homelessCollection.homeSetId = null
db.collectionDao().insertOrUpdateByUrlAndRememberFlags(homelessCollection)
collectionRepository.insertOrUpdateByUrlAndRememberFlags(homelessCollection)
}
}
@ -245,7 +247,7 @@ class CollectionListRefresher @AssistedInject constructor(
for((url, localCollection) in homelessCollections) try {
DavResource(httpClient, url).propfind(0, *RefreshCollectionsWorker.DAV_COLLECTION_PROPERTIES) { response, _ ->
if (!response.isSuccess()) {
db.collectionDao().delete(localCollection)
collectionRepository.delete(localCollection)
return@propfind
}
@ -264,13 +266,13 @@ class CollectionListRefresher @AssistedInject constructor(
collection.ownerId = principalId
}
db.collectionDao().insertOrUpdateByUrlAndRememberFlags(collection)
} ?: db.collectionDao().delete(localCollection)
collectionRepository.insertOrUpdateByUrlAndRememberFlags(collection)
} ?: collectionRepository.delete(localCollection)
}
} catch (e: HttpException) {
// delete collection locally if it was not accessible (40x)
if (e.code in arrayOf(403, 404, 410))
db.collectionDao().delete(localCollection)
collectionRepository.delete(localCollection)
else
throw e
}

View file

@ -76,7 +76,7 @@ class CollectionScreenModel @AssistedInject constructor(
inProgress = true
noCancellationScope.launch {
try {
collectionRepository.delete(collection)
collectionRepository.deleteRemote(collection)
} catch (e: Exception) {
error = e
} finally {