Use ID to match DB collections with content provider collections

app/src/androidTest/kotlin/at/bitfire/davdroid/resource/LocalAddressBookStoreTest.kt

@@ -9,26 +9,18 @@ import android.accounts.AccountManager
 import android.content.ContentProviderClient
 import android.content.Context
 import at.bitfire.davdroid.R
+import at.bitfire.davdroid.db.AppDatabase
 import at.bitfire.davdroid.db.Collection
 import at.bitfire.davdroid.db.Service
-import at.bitfire.davdroid.repository.DavCollectionRepository
-import at.bitfire.davdroid.repository.DavServiceRepository
-import at.bitfire.davdroid.settings.SettingsManager
-import at.bitfire.davdroid.sync.account.SystemAccountUtils
+import at.bitfire.davdroid.sync.account.TestAccount
 import dagger.hilt.android.qualifiers.ApplicationContext
 import dagger.hilt.android.testing.HiltAndroidRule
 import dagger.hilt.android.testing.HiltAndroidTest
-import io.mockk.MockKAnnotations
 import io.mockk.every
-import io.mockk.impl.annotations.InjectMockKs
 import io.mockk.impl.annotations.RelaxedMockK
-import io.mockk.impl.annotations.SpyK
-import io.mockk.just
+import io.mockk.junit4.MockKRule
 import io.mockk.mockk
 import io.mockk.mockkObject
-import io.mockk.runs
-import io.mockk.unmockkAll
-import io.mockk.verify
 import okhttp3.HttpUrl.Companion.toHttpUrl
 import org.junit.After
 import org.junit.Assert.assertEquals
@@ -37,71 +29,59 @@ import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
-import java.util.logging.Logger
 import javax.inject.Inject
 
 @HiltAndroidTest
 class LocalAddressBookStoreTest {
 
-    @get:Rule
-    val hiltRule = HiltAndroidRule(this)
-
-    @Inject
-    @ApplicationContext
-    @SpyK
+    @Inject @ApplicationContext
     lateinit var context: Context
 
-    val account by lazy { Account("Test Account", context.getString(R.string.account_type)) }
-    val addressBookAccount by lazy { Account("[email protected]", context.getString(R.string.account_type_address_book)) }
-
-    val provider = mockk<ContentProviderClient>(relaxed = true)
-    val addressBook: LocalAddressBook = mockk(relaxed = true) {
-        every { account } answers { [email protected] }
-        every { updateSyncFrameworkSettings() } just runs
-        every { addressBookAccount } answers { [email protected] }
-        every { settings } returns LocalAddressBookStore.contactsProviderSettings
-    }
-
-    @Suppress("unused")     // used by @InjectMockKs LocalAddressBookStore
-    @RelaxedMockK
-    lateinit var collectionRepository: DavCollectionRepository
-
-    @Suppress("unused")     // used by @InjectMockKs LocalAddressBookStore
-    val localAddressBookFactory = mockk<LocalAddressBook.Factory> {
-        every { create(any(), any(), provider) } returns addressBook
-    }
+    @Inject
+    lateinit var db: AppDatabase
 
     @Inject
-    @SpyK
-    lateinit var logger: Logger
+    lateinit var localAddressBookStore: LocalAddressBookStore
 
     @RelaxedMockK
-    lateinit var settingsManager: SettingsManager
+    lateinit var provider: ContentProviderClient
 
-    @Suppress("unused")     // used by @InjectMockKs LocalAddressBookStore
-    val serviceRepository = mockk<DavServiceRepository>(relaxed = true) {
-        every { get(any<Long>()) } returns null
-        every { get(200) } returns mockk<Service> {
-            every { accountName } returns "[email protected]"
-        }
-    }
+    @get:Rule
+    val hiltRule = HiltAndroidRule(this)
 
-    @InjectMockKs
-    @SpyK
-    lateinit var localAddressBookStore: LocalAddressBookStore
+    @get:Rule
+    val mockkRule = MockKRule(this)
+
+    lateinit var addressBookAccountType: String
 
+    lateinit var addressBookAccount: Account
+    lateinit var account: Account
+    lateinit var service: Service
 
     @Before
     fun setUp() {
         hiltRule.inject()
 
-        // initialize global mocks
-        MockKAnnotations.init(this)
+        addressBookAccountType = context.getString(R.string.account_type_address_book)
+        removeAddressBooks()
+
+        account = TestAccount.create()
+        service = Service(
+            id = 200,
+            accountName = account.name,
+            type = Service.Companion.TYPE_CARDDAV,
+            principal = null
+        )
+        db.serviceDao().insertOrReplace(service)
+        addressBookAccount = Account(
+            "[email protected]",
+            addressBookAccountType
+        )
     }
 
     @After
     fun tearDown() {
-        unmockkAll()
+        TestAccount.remove(account)
     }
 
 
@@ -111,7 +91,7 @@ class LocalAddressBookStoreTest {
             every { id } returns 42
             every { url } returns "https://example.com/addressbook/funnyfriends".toHttpUrl()
             every { displayName } returns null
-            every { serviceId } returns 404
+            every { serviceId } returns 404     // missing service
         }
         assertEquals("funnyfriends #42", localAddressBookStore.accountName(collection))
     }
@@ -122,19 +102,19 @@ class LocalAddressBookStoreTest {
             every { id } returns 42
             every { url } returns "https://example.com/addressbook/funnyfriends".toHttpUrl()
             every { displayName } returns null
-            every { serviceId } returns 200
+            every { serviceId } returns service.id
         }
         val accountName = localAddressBookStore.accountName(collection)
-        assertEquals("funnyfriends ([email protected]) #42", accountName)
+        assertEquals("funnyfriends (${account.name}) #42", accountName)
     }
 
     @Test
     fun test_accountName_missingDisplayNameAndService() {
-        val collection = mockk<Collection>(relaxed = true) {
+        val collection = mockk<Collection> {
             every { id } returns 1
             every { url } returns "https://example.com/addressbook/funnyfriends".toHttpUrl()
             every { displayName } returns null
-            every { serviceId } returns 404 // missing service
+            every { serviceId } returns 404     // missing service
         }
         assertEquals("funnyfriends #1", localAddressBookStore.accountName(collection))
     }
@@ -143,45 +123,42 @@ class LocalAddressBookStoreTest {
     @Test
     fun test_create_createAccountReturnsNull() {
         val collection = mockk<Collection>(relaxed = true) {
-            every { serviceId } returns 200
+            every { serviceId } returns service.id
             every { id } returns 1
             every { url } returns "https://example.com/addressbook/funnyfriends".toHttpUrl()
         }
-        every { localAddressBookStore.createAddressBookAccount(any(), any(), any(), any()) } returns null
+
+        mockkObject(localAddressBookStore)
+        every { localAddressBookStore.createAddressBookAccount(any(), any(), any()) } returns null
+
         assertEquals(null, localAddressBookStore.create(provider, collection))
     }
 
     @Test
-    fun test_create_createAccountReturnsAccount() {
+    fun test_create_ReadOnly() {
         val collection = mockk<Collection>(relaxed = true) {
-            every { serviceId } returns 200
+            every { serviceId } returns service.id
             every { id } returns 1
             every { url } returns "https://example.com/addressbook/funnyfriends".toHttpUrl()
+            every { readOnly() } returns true
         }
-        every { localAddressBookStore.createAddressBookAccount(any(), any(), any(), any()) } returns addressBookAccount
-        every { addressBook.readOnly } returns true
         val addrBook = localAddressBookStore.create(provider, collection)!!
-
-        verify(exactly = 1) { addressBook.updateSyncFrameworkSettings() }
-        assertEquals(addressBookAccount, addrBook.addressBookAccount)
-        assertEquals(LocalAddressBookStore.contactsProviderSettings, addrBook.settings)
-        assertEquals(true, addrBook.readOnly)
-
-        every { addressBook.readOnly } returns false
-        val addrBook2 = localAddressBookStore.create(provider, collection)!!
-        assertEquals(false, addrBook2.readOnly)
+        assertEquals(Account("funnyfriends (Test Account) #1", addressBookAccountType), addrBook.addressBookAccount)
+        assertTrue(addrBook.readOnly)
     }
 
     @Test
-    fun test_createAccount_succeeds() {
-        mockkObject(SystemAccountUtils)
-        every { SystemAccountUtils.createAccount(any(), any(), any()) } returns true
-        val result: Account = localAddressBookStore.createAddressBookAccount(
-            account, "[email protected]", 42, "https://example.com/addressbook/funnyfriends"
-        )!!
-        verify(exactly = 1) { SystemAccountUtils.createAccount(any(), any(), any()) }
-        assertEquals("[email protected]", result.name)
-        assertEquals(context.getString(R.string.account_type_address_book), result.type)
+    fun test_create_ReadWrite() {
+        val collection = mockk<Collection>(relaxed = true) {
+            every { serviceId } returns service.id
+            every { id } returns 1
+            every { url } returns "https://example.com/addressbook/funnyfriends".toHttpUrl()
+            every { readOnly() } returns false
+        }
+
+        val addrBook = localAddressBookStore.create(provider, collection)!!
+        assertEquals(Account("funnyfriends (Test Account) #1", addressBookAccountType), addrBook.addressBookAccount)
+        assertFalse(addrBook.readOnly)
     }
 
 
@@ -223,4 +200,14 @@ class LocalAddressBookStoreTest {
         assertTrue(LocalAddressBookStore.shouldBeReadOnly(collectionNotReadOnly, true))
     }
 
+
+    // helpers
+
+    private fun removeAddressBooks() {
+        val accountManager = AccountManager.get(context)
+        accountManager.getAccountsByType(addressBookAccountType).forEach {
+            accountManager.removeAccount(it, null, null)
+        }
+    }
+
 }

app/src/androidTest/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration17Test.kt

@@ -53,6 +53,7 @@ class AccountSettingsMigration17Test {
 
     @Test
     fun testMigrate_OldAddressBook_CollectionInDB() {
+        val localAddressBookUserDataUrl = "url"
         TestAccount.provide(version = 16) { account ->
             val accountManager = AccountManager.get(context)
             val addressBookAccountType = context.getString(R.string.account_type_address_book)
@@ -63,7 +64,7 @@ class AccountSettingsMigration17Test {
                 // address book has account + URL
                 val url = "https://example.com/address-book"
                 accountManager.setAndVerifyUserData(addressBookAccount, "real_account_name", account.name)
-                accountManager.setAndVerifyUserData(addressBookAccount, LocalAddressBook.USER_DATA_URL, url)
+                accountManager.setAndVerifyUserData(addressBookAccount, localAddressBookUserDataUrl, url)
 
                 // and is known in database
                 db.serviceDao().insertOrReplace(
@@ -86,7 +87,7 @@ class AccountSettingsMigration17Test {
 
                 // migration renames address book, update account
                 addressBookAccount = accountManager.getAccountsByType(addressBookAccountType).filter {
-                    accountManager.getUserData(it, LocalAddressBook.USER_DATA_URL) == url
+                    accountManager.getUserData(it, localAddressBookUserDataUrl) == url
                 }.first()
                 assertEquals("Some Address Book (${account.name}) #100", addressBookAccount.name)
 

app/src/androidTest/kotlin/at/bitfire/davdroid/sync/LocalTestCollection.kt

@@ -8,7 +8,7 @@ import at.bitfire.davdroid.db.SyncState
 import at.bitfire.davdroid.resource.LocalCollection
 
 class LocalTestCollection(
-    override val collectionUrl: String = "http://example.com/test/"
+    override val dbCollectionId: Long = 0L
 ): LocalCollection<LocalTestResource> {
 
     override val tag = "LocalTestCollection"

app/src/androidTest/kotlin/at/bitfire/davdroid/sync/SyncerTest.kt

@@ -17,7 +17,6 @@ import io.mockk.just
 import io.mockk.mockk
 import io.mockk.runs
 import io.mockk.verify
-import okhttp3.HttpUrl.Companion.toHttpUrl
 import org.junit.Assert.assertArrayEquals
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
@@ -66,9 +65,10 @@ class SyncerTest {
 
     @Test
     fun testUpdateCollections_deletesCollection() {
-        val localCollection = mockk<LocalTestCollection>()
-        every { localCollection.collectionUrl } returns "http://delete.the/collection"
-        every { localCollection.title } returns "Collection to be deleted locally"
+        val localCollection = mockk<LocalTestCollection> {
+            every { dbCollectionId } returns 0L
+            every { title } returns "Collection to be deleted locally"
+        }
 
         // Should delete the localCollection if dbCollection (remote) does not exist
         val localCollections = mutableListOf(localCollection)
@@ -81,12 +81,14 @@ class SyncerTest {
 
     @Test
     fun testUpdateCollections_updatesCollection() {
-        val localCollection = mockk<LocalTestCollection>()
-        val dbCollection = mockk<Collection>()
-        val dbCollections = mapOf("http://update.the/collection".toHttpUrl() to dbCollection)
-        every { dbCollection.url } returns "http://update.the/collection".toHttpUrl()
-        every { localCollection.collectionUrl } returns "http://update.the/collection"
-        every { localCollection.title } returns "The Local Collection"
+        val localCollection = mockk<LocalTestCollection> {
+            every { dbCollectionId } returns 0L
+            every { title } returns "The Local Collection"
+        }
+        val dbCollection = mockk<Collection> {
+            every { id } returns 0L
+        }
+        val dbCollections = mapOf(0L to dbCollection)
 
         // Should update the localCollection if it exists
         val result = syncer.updateCollections(provider, listOf(localCollection), dbCollections)
@@ -99,21 +101,21 @@ class SyncerTest {
     @Test
     fun testUpdateCollections_findsNewCollection() {
         val dbCollection = mockk<Collection> {
-            every { url } returns "http://newly.found/collection".toHttpUrl()
+            every { id } returns 0L
         }
         val localCollections = listOf(mockk<LocalTestCollection> {
-            every { collectionUrl } returns "http://newly.found/collection"
+            every { dbCollectionId } returns 0L
         })
         val dbCollections = listOf(dbCollection)
-        val dbCollectionsMap = mapOf(dbCollection.url to dbCollection)
+        val dbCollectionsMap = mapOf(dbCollection.id to dbCollection)
         every { syncer.createLocalCollections(provider, dbCollections) } returns localCollections
 
         // Should return the new collection, because it was not updated
         val result = syncer.updateCollections(provider, emptyList(), dbCollectionsMap)
 
         // Updated local collection list contain new entry
         assertEquals(1, result.size)
-        assertEquals(dbCollection.url.toString(), result[0].collectionUrl)
+        assertEquals(dbCollection.id, result[0].dbCollectionId)
     }
 
 
@@ -134,14 +136,14 @@ class SyncerTest {
         val dbCollection1 = mockk<Collection>()
         val dbCollection2 = mockk<Collection>()
         val dbCollections = mapOf(
-            "http://newly.found/collection1".toHttpUrl() to dbCollection1,
-            "http://newly.found/collection2".toHttpUrl() to dbCollection2
+            0L to dbCollection1,
+            1L to dbCollection2
         )
-        val localCollection1 = mockk<LocalTestCollection>()
-        val localCollection2 = mockk<LocalTestCollection>()
+        val localCollection1 = mockk<LocalTestCollection> { every { dbCollectionId } returns 0L }
+        val localCollection2 = mockk<LocalTestCollection> { every { dbCollectionId } returns 1L }
         val localCollections = listOf(localCollection1, localCollection2)
-        every { localCollection1.collectionUrl } returns "http://newly.found/collection1"
-        every { localCollection2.collectionUrl } returns "http://newly.found/collection2"
+        every { localCollection1.dbCollectionId } returns 0L
+        every { localCollection2.dbCollectionId } returns 1L
         every { syncer.syncCollection(provider, any(), any()) } just runs
 
         // Should call the collection content sync on both collections