Move Insert/update to DAO (#1587)

* Move homeset insert/update logic from repository to DAO; add thread-safety test

* Rename insertOrUpdateByUrlRememberSync

app/src/androidTest/kotlin/at/bitfire/davdroid/db/HomeSetDaoTest.kt

@@ -0,0 +1,97 @@
+/*
+ * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
+ */
+
+package at.bitfire.davdroid.db
+
+import dagger.hilt.android.testing.HiltAndroidRule
+import dagger.hilt.android.testing.HiltAndroidTest
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.runBlocking
+import okhttp3.HttpUrl.Companion.toHttpUrl
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Before
+import org.junit.Rule
+import org.junit.Test
+import javax.inject.Inject
+
+@HiltAndroidTest
+class HomeSetDaoTest {
+
+    @get:Rule
+    var hiltRule = HiltAndroidRule(this)
+
+    @Inject
+    lateinit var db: AppDatabase
+    lateinit var dao: HomeSetDao
+    var serviceId: Long = 0
+
+    @Before
+    fun setUp() {
+        hiltRule.inject()
+        dao = db.homeSetDao()
+
+        serviceId = db.serviceDao().insertOrReplace(
+            Service(id=0, accountName="test", type= Service.TYPE_CALDAV, principal = null)
+        )
+    }
+
+    @After
+    fun tearDown() {
+        db.serviceDao().deleteAll()
+    }
+
+
+    @Test
+    fun testInsertOrUpdate() {
+        // should insert new row or update (upsert) existing row - without changing its key!
+        val entry1 = HomeSet(id=0, serviceId=serviceId, personal=true, url="https://example.com/1".toHttpUrl())
+        val insertId1 = dao.insertOrUpdateByUrlBlocking(entry1)
+        assertEquals(1L, insertId1)
+        assertEquals(entry1.copy(id = 1L), dao.getById(1))
+
+        val updatedEntry1 = HomeSet(id=0, serviceId=serviceId, personal=true, url="https://example.com/1".toHttpUrl(), displayName="Updated Entry")
+        val updateId1 = dao.insertOrUpdateByUrlBlocking(updatedEntry1)
+        assertEquals(1L, updateId1)
+        assertEquals(updatedEntry1.copy(id = 1L), dao.getById(1))
+
+        val entry2 = HomeSet(id=0, serviceId=serviceId, personal=true, url= "https://example.com/2".toHttpUrl())
+        val insertId2 = dao.insertOrUpdateByUrlBlocking(entry2)
+        assertEquals(2L, insertId2)
+        assertEquals(entry2.copy(id = 2L), dao.getById(2))
+    }
+
+    @Test
+    fun testInsertOrUpdate_TransactionSafe() {
+        runBlocking(Dispatchers.IO) {
+            for (i in 0..9999)
+                launch {
+                    dao.insertOrUpdateByUrlBlocking(
+                        HomeSet(
+                            id = 0,
+                            serviceId = serviceId,
+                            url = "https://example.com/".toHttpUrl(),
+                            personal = true
+                        )
+                    )
+                }
+        }
+        assertEquals(1, dao.getByService(serviceId).size)
+    }
+
+    @Test
+    fun testDelete() {
+        // should delete row with given primary key (id)
+        val entry1 = HomeSet(id=1, serviceId=serviceId, personal=true, url= "https://example.com/1".toHttpUrl())
+
+        val insertId1 = dao.insertOrUpdateByUrlBlocking(entry1)
+        assertEquals(1L, insertId1)
+        assertEquals(entry1, dao.getById(1L))
+
+        dao.delete(entry1)
+        assertEquals(null, dao.getById(1L))
+    }
+
+}

app/src/androidTest/kotlin/at/bitfire/davdroid/db/SyncStatsDaoTest.kt

@@ -0,0 +1,77 @@
+/*
+ * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
+ */
+
+package at.bitfire.davdroid.db
+
+import androidx.sqlite.SQLiteException
+import at.bitfire.davdroid.sync.SyncDataType
+import dagger.hilt.android.testing.HiltAndroidRule
+import dagger.hilt.android.testing.HiltAndroidTest
+import kotlinx.coroutines.test.runTest
+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 SyncStatsDaoTest {
+
+    @get:Rule
+    val hiltRule = HiltAndroidRule(this)
+
+    @Inject
+    lateinit var db: AppDatabase
+    var collectionId: Long = 0
+
+    @Before
+    fun setUp() {
+        hiltRule.inject()
+
+        val serviceId = db.serviceDao().insertOrReplace(Service(
+            id = 0,
+            accountName = "test@example.com",
+            type = Service.TYPE_CALDAV
+        ))
+        collectionId = db.collectionDao().insert(Collection(
+            id = 0,
+            serviceId = serviceId,
+            type = Collection.TYPE_CALENDAR,
+            url = "https://example.com".toHttpUrl()
+        ))
+    }
+
+    @After
+    fun tearDown() {
+        db.serviceDao().deleteAll()
+    }
+
+    @Test
+    fun testInsertOrReplace_ExistingForeignKey() = runTest {
+        val dao = db.syncStatsDao()
+        dao.insertOrReplace(
+            SyncStats(
+                id = 0,
+                collectionId = collectionId,
+                dataType = SyncDataType.CONTACTS.toString(),
+                lastSync = System.currentTimeMillis()
+            )
+        )
+    }
+
+    @Test(expected = SQLiteException::class)
+    fun testInsertOrReplace_MissingForeignKey() = runTest {
+        val dao = db.syncStatsDao()
+        dao.insertOrReplace(
+            SyncStats(
+                id = 0,
+                collectionId = 12345,
+                dataType = SyncDataType.CONTACTS.toString(),
+                lastSync = System.currentTimeMillis()
+            )
+        )
+    }
+
+}

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

@@ -1,74 +0,0 @@
-/*
- * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
- */
-
-package at.bitfire.davdroid.repository
-
-import at.bitfire.davdroid.db.HomeSet
-import at.bitfire.davdroid.db.Service
-import dagger.hilt.android.testing.HiltAndroidRule
-import dagger.hilt.android.testing.HiltAndroidTest
-import okhttp3.HttpUrl.Companion.toHttpUrl
-import org.junit.Assert.assertEquals
-import org.junit.Before
-import org.junit.Rule
-import org.junit.Test
-import javax.inject.Inject
-
-@HiltAndroidTest
-class DavHomeSetRepositoryTest {
-
-    @Inject
-    lateinit var repository: DavHomeSetRepository
-
-    @Inject
-    lateinit var serviceRepository: DavServiceRepository
-
-    @get:Rule
-    var hiltRule = HiltAndroidRule(this)
-
-    var serviceId: Long = 0
-
-    @Before
-    fun setUp() {
-        hiltRule.inject()
-
-        serviceId = serviceRepository.insertOrReplaceBlocking(
-            Service(id=0, accountName="test", type= Service.TYPE_CALDAV, principal = null)
-        )
-    }
-
-
-    @Test
-    fun testInsertOrUpdate() {
-        // should insert new row or update (upsert) existing row - without changing its key!
-        val entry1 = HomeSet(id=0, serviceId=serviceId, personal=true, url="https://example.com/1".toHttpUrl())
-        val insertId1 = repository.insertOrUpdateByUrlBlocking(entry1)
-        assertEquals(1L, insertId1)
-        assertEquals(entry1.copy(id = 1L), repository.getByIdBlocking(1L))
-
-        val updatedEntry1 = HomeSet(id=0, serviceId=serviceId, personal=true, url="https://example.com/1".toHttpUrl(), displayName="Updated Entry")
-        val updateId1 = repository.insertOrUpdateByUrlBlocking(updatedEntry1)
-        assertEquals(1L, updateId1)
-        assertEquals(updatedEntry1.copy(id = 1L), repository.getByIdBlocking(1L))
-
-        val entry2 = HomeSet(id=0, serviceId=serviceId, personal=true, url= "https://example.com/2".toHttpUrl())
-        val insertId2 = repository.insertOrUpdateByUrlBlocking(entry2)
-        assertEquals(2L, insertId2)
-        assertEquals(entry2.copy(id = 2L), repository.getByIdBlocking(2L))
-    }
-
-    @Test
-    fun testDeleteBlocking() {
-        // should delete row with given primary key (id)
-        val entry1 = HomeSet(id=1, serviceId=serviceId, personal=true, url= "https://example.com/1".toHttpUrl())
-
-        val insertId1 = repository.insertOrUpdateByUrlBlocking(entry1)
-        assertEquals(1L, insertId1)
-        assertEquals(entry1, repository.getByIdBlocking(1L))
-
-        repository.deleteBlocking(entry1)
-        assertEquals(null, repository.getByIdBlocking(1L))
-    }
-
-}

app/src/main/kotlin/at/bitfire/davdroid/db/HomeSetDao.kt

@@ -8,6 +8,7 @@ import androidx.room.Dao
 import androidx.room.Delete
 import androidx.room.Insert
 import androidx.room.Query
+import androidx.room.Transaction
 import androidx.room.Update
 import kotlinx.coroutines.flow.Flow
 
@@ -35,6 +36,24 @@ interface HomeSetDao {
     @Update
     fun update(homeset: HomeSet)
 
+    /**
+     * If a homeset with the given service ID and URL already exists, it is updated with the other fields.
+     * Otherwise, a new homeset is inserted.
+     *
+     * This method preserves the primary key, as opposed to using "@Insert(onConflict = OnConflictStrategy.REPLACE)"
+     * which will create a new row with incremented ID and thus breaks entity relationships!
+     *
+     * @param homeSet   home set to insert/update
+     *
+     * @return ID of the row that has been inserted or updated. -1 If the insert fails due to other reasons.
+     */
+    @Transaction
+    fun insertOrUpdateByUrlBlocking(homeSet: HomeSet): Long =
+        getByUrl(homeSet.serviceId, homeSet.url.toString())?.let { existingHomeset ->
+            update(homeSet.copy(id = existingHomeset.id))
+            existingHomeset.id
+        } ?: insert(homeSet)
+
     @Delete
     fun delete(homeset: HomeSet)
 

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

@@ -228,7 +228,7 @@ class DavCollectionRepository @Inject constructor(
      *
      * @param newCollection Collection to be inserted or updated
      */
-    fun insertOrUpdateByUrlAndRememberFlags(newCollection: Collection) {
+    fun insertOrUpdateByUrlRememberSync(newCollection: Collection) {
         db.runInTransaction {
             // remember locally set flags
             val oldCollection = dao.getByServiceAndUrl(newCollection.serviceId, newCollection.url.toString())

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

@@ -5,7 +5,6 @@
 package at.bitfire.davdroid.repository
 
 import android.accounts.Account
-import androidx.room.Transaction
 import at.bitfire.davdroid.db.AppDatabase
 import at.bitfire.davdroid.db.HomeSet
 import at.bitfire.davdroid.db.Service
@@ -29,21 +28,8 @@ class DavHomeSetRepository @Inject constructor(
     fun getCalendarHomeSetsFlow(account: Account) =
         dao.getBindableByAccountAndServiceTypeFlow(account.name, Service.TYPE_CALDAV)
 
-
-    /**
-     * Tries to insert new row, but updates existing row if already present.
-     * This method preserves the primary key, as opposed to using "@Insert(onConflict = OnConflictStrategy.REPLACE)"
-     * which will create a new row with incremented ID and thus breaks entity relationships!
-     *
-     * @return ID of the row, that has been inserted or updated. -1 If the insert fails due to other reasons.
-     */
-    @Transaction
-    fun insertOrUpdateByUrlBlocking(homeset: HomeSet): Long =
-        dao.getByUrl(homeset.serviceId, homeset.url.toString())?.let { existingHomeset ->
-            dao.update(homeset.copy(id = existingHomeset.id))
-            existingHomeset.id
-        } ?: dao.insert(homeset)
-
+    fun insertOrUpdateByUrlBlocking(homeSet: HomeSet): Long =
+        dao.insertOrUpdateByUrlBlocking(homeSet)
 
     fun deleteBlocking(homeSet: HomeSet) = dao.delete(homeSet)
 

app/src/main/kotlin/at/bitfire/davdroid/servicedetection/CollectionListRefresher.kt

@@ -279,7 +279,7 @@ class CollectionListRefresher @AssistedInject constructor(
 
                     // save or update collection if usable (ignore it otherwise)
                     if (isUsableCollection(collection))
-                        collectionRepository.insertOrUpdateByUrlAndRememberFlags(collection)
+                        collectionRepository.insertOrUpdateByUrlRememberSync(collection)
 
                     // Remove this collection from queue - because it was found in the home set
                     localHomesetCollections.remove(collection.url)
@@ -292,7 +292,7 @@ class CollectionListRefresher @AssistedInject constructor(
 
             // Mark leftover (not rediscovered) collections from queue as homeless (remove association)
             for ((_, homelessCollection) in localHomesetCollections)
-                collectionRepository.insertOrUpdateByUrlAndRememberFlags(
+                collectionRepository.insertOrUpdateByUrlRememberSync(
                     homelessCollection.copy(homeSetId = null)
                 )
 
@@ -317,7 +317,7 @@ class CollectionListRefresher @AssistedInject constructor(
                 Collection.fromDavResponse(response)?.let { collection ->
                     if (!isUsableCollection(collection))
                         return@let
-                    collectionRepository.insertOrUpdateByUrlAndRememberFlags(collection.copy(
+                    collectionRepository.insertOrUpdateByUrlRememberSync(collection.copy(
                         serviceId = localCollection.serviceId,          // use same service ID as previous entry
                         ownerId = response[Owner::class.java]?.href     // save the principal id (collection owner)
                             ?.let { response.href.resolve(it) }