From a43d493252a44149984f1d0763b8fc62799fa415 Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Mon, 30 Dec 2024 17:45:35 +0100 Subject: [PATCH] Fix tests --- .../repository/AccountRepositoryTest.kt | 56 ++++++++++++++++ .../repository/DavCollectionRepositoryTest.kt | 23 ++++--- .../repository/DavHomeSetRepositoryTest.kt | 33 +++++++--- .../AccountSettingsMigration17Test.kt | 2 + .../sync/account/AccountsCleanupWorkerTest.kt | 66 +++---------------- .../sync/account/TestAccountAuthenticator.kt | 7 +- .../at/bitfire/davdroid/db/AccountDao.kt | 8 +++ .../at/bitfire/davdroid/db/ServiceDao.kt | 3 - .../davdroid/repository/AccountRepository.kt | 8 +++ .../repository/DavServiceRepository.kt | 3 - .../sync/account/AccountsCleanupWorker.kt | 20 +----- 11 files changed, 123 insertions(+), 106 deletions(-) create mode 100644 app/src/androidTest/kotlin/at/bitfire/davdroid/repository/AccountRepositoryTest.kt diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/repository/AccountRepositoryTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/repository/AccountRepositoryTest.kt new file mode 100644 index 000000000..dd20707b6 --- /dev/null +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/repository/AccountRepositoryTest.kt @@ -0,0 +1,56 @@ +/* + * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details. + */ + +package at.bitfire.davdroid.repository + +import android.content.Context +import at.bitfire.davdroid.R +import at.bitfire.davdroid.db.AppDatabase +import at.bitfire.davdroid.sync.account.TestAccountAuthenticator +import dagger.hilt.android.qualifiers.ApplicationContext +import dagger.hilt.android.testing.HiltAndroidRule +import dagger.hilt.android.testing.HiltAndroidTest +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import javax.inject.Inject +import at.bitfire.davdroid.db.Account as DbAccount + +@HiltAndroidTest +class AccountRepositoryTest { + + @Inject + lateinit var accountRepository: AccountRepository + + @Inject @ApplicationContext + lateinit var context: Context + + @Inject + lateinit var db: AppDatabase + + @get:Rule + val hiltRule = HiltAndroidRule(this) + + @Before + fun setUp() { + hiltRule.inject() + } + + + @Test + fun testRemoveOrphanedInDb() { + TestAccountAuthenticator.provide(accountType = context.getString(R.string.account_type)) { systemAccount -> + val dao = db.accountDao() + dao.insertOrIgnore(DbAccount(id = 1, name = systemAccount.name)) + dao.insertOrIgnore(DbAccount(id = 2, name = "no-corresponding-system-account")) + + accountRepository.removeOrphanedInDb() + + // now the account without a corresponding system account should be removed + assertEquals(listOf(DbAccount(id = 1, name = systemAccount.name)), db.accountDao().getAll()) + } + } + +} \ No newline at end of file diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/repository/DavCollectionRepositoryTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/repository/DavCollectionRepositoryTest.kt index 65fd1e167..ffcefa9a6 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/repository/DavCollectionRepositoryTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/repository/DavCollectionRepositoryTest.kt @@ -1,10 +1,12 @@ package at.bitfire.davdroid.repository +import android.accounts.Account import android.content.Context import at.bitfire.davdroid.db.AppDatabase import at.bitfire.davdroid.db.Collection import at.bitfire.davdroid.db.Service import at.bitfire.davdroid.settings.AccountSettings +import at.bitfire.davdroid.sync.account.TestAccountAuthenticator import dagger.Lazy import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.android.testing.HiltAndroidRule @@ -18,6 +20,7 @@ import org.junit.Before import org.junit.Rule import org.junit.Test import javax.inject.Inject +import at.bitfire.davdroid.db.Account as DbAccount @HiltAndroidTest class DavCollectionRepositoryTest { @@ -41,18 +44,26 @@ class DavCollectionRepositoryTest { @Inject lateinit var serviceRepository: DavServiceRepository - var service: Service? = null + lateinit var account: Account + var serviceId: Long = 0L @Before fun setUp() { hiltRule.inject() - service = createTestService(Service.TYPE_CARDDAV)!! + + account = TestAccountAuthenticator.create() + db.accountDao().insertOrIgnore(DbAccount(name = account.name)) + + val service = Service(id=0, accountName=account.name, type= Service.TYPE_CALDAV, principal = null) + serviceId = serviceRepository.insertOrReplace(service) } @After fun cleanUp() { db.close() + serviceRepository.deleteAll() + TestAccountAuthenticator.remove(account) } @@ -60,7 +71,7 @@ class DavCollectionRepositoryTest { fun testOnChangeListener_setForceReadOnly() = runBlocking { val collectionId = db.collectionDao().insertOrUpdateByUrl( Collection( - serviceId = service!!.id, + serviceId = serviceId, type = Collection.TYPE_ADDRESSBOOK, url = "https://example.com".toHttpUrl(), forceReadOnly = false, @@ -94,10 +105,4 @@ class DavCollectionRepositoryTest { // Test helpers and dependencies - private fun createTestService(serviceType: String) : Service? { - val service = Service(id=0, accountName="test", type=serviceType, principal = null) - val serviceId = serviceRepository.insertOrReplace(service) - return serviceRepository.get(serviceId) - } - } \ No newline at end of file diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/repository/DavHomeSetRepositoryTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/repository/DavHomeSetRepositoryTest.kt index 1af05be25..94a772147 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/repository/DavHomeSetRepositoryTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/repository/DavHomeSetRepositoryTest.kt @@ -4,16 +4,21 @@ package at.bitfire.davdroid.repository +import android.accounts.Account +import at.bitfire.davdroid.db.AppDatabase import at.bitfire.davdroid.db.HomeSet import at.bitfire.davdroid.db.Service +import at.bitfire.davdroid.sync.account.TestAccountAuthenticator import dagger.hilt.android.testing.HiltAndroidRule import dagger.hilt.android.testing.HiltAndroidTest 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 +import at.bitfire.davdroid.db.Account as DbAccount @HiltAndroidTest class DavHomeSetRepositoryTest { @@ -21,23 +26,39 @@ class DavHomeSetRepositoryTest { @get:Rule var hiltRule = HiltAndroidRule(this) + @Inject + lateinit var db: AppDatabase + @Inject lateinit var repository: DavHomeSetRepository @Inject lateinit var serviceRepository: DavServiceRepository + lateinit var account: Account + var serviceId: Long = 0L + + @Before fun setUp() { hiltRule.inject() + + account = TestAccountAuthenticator.create() + db.accountDao().insertOrIgnore(DbAccount(name = account.name)) + + val service = Service(id=0, accountName=account.name, type= Service.TYPE_CALDAV, principal = null) + serviceId = serviceRepository.insertOrReplace(service) + } + + @After + fun tearDown() { + TestAccountAuthenticator.remove(account) } @Test fun testInsertOrUpdate() { // should insert new row or update (upsert) existing row - without changing its key! - val serviceId = createTestService() - val entry1 = HomeSet(id=0, serviceId=serviceId, personal=true, url="https://example.com/1".toHttpUrl()) val insertId1 = repository.insertOrUpdateByUrl(entry1) assertEquals(1L, insertId1) @@ -57,8 +78,6 @@ class DavHomeSetRepositoryTest { @Test fun testDelete() { // should delete row with given primary key (id) - val serviceId = createTestService() - val entry1 = HomeSet(id=1, serviceId=serviceId, personal=true, url= "https://example.com/1".toHttpUrl()) val insertId1 = repository.insertOrUpdateByUrl(entry1) @@ -69,10 +88,4 @@ class DavHomeSetRepositoryTest { assertEquals(null, repository.getById(1L)) } - - private fun createTestService() : Long { - val service = Service(id=0, accountName="test", type= Service.TYPE_CALDAV, principal = null) - return serviceRepository.insertOrReplace(service) - } - } \ No newline at end of file diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration17Test.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration17Test.kt index 2e4d18630..2132ac4c0 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration17Test.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration17Test.kt @@ -25,6 +25,7 @@ import org.junit.Before import org.junit.Rule import org.junit.Test import javax.inject.Inject +import at.bitfire.davdroid.db.Account as DbAccount @HiltAndroidTest class AccountSettingsMigration17Test { @@ -66,6 +67,7 @@ class AccountSettingsMigration17Test { accountManager.setAndVerifyUserData(addressBookAccount, LocalAddressBook.USER_DATA_URL, url) // and is known in database + db.accountDao().insertOrIgnore(DbAccount(name = account.name)) db.serviceDao().insertOrReplace( Service( id = 1, accountName = account.name, type = Service.TYPE_CARDDAV, principal = null diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/account/AccountsCleanupWorkerTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/account/AccountsCleanupWorkerTest.kt index 1bd531e23..1db7cc896 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/account/AccountsCleanupWorkerTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/account/AccountsCleanupWorkerTest.kt @@ -15,17 +15,14 @@ import at.bitfire.davdroid.settings.SettingsManager import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.android.testing.HiltAndroidRule import dagger.hilt.android.testing.HiltAndroidTest -import io.mockk.every -import io.mockk.mockkObject import org.junit.After import org.junit.Assert.assertEquals -import org.junit.Assert.assertNotNull -import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Rule import org.junit.Test import javax.inject.Inject +import at.bitfire.davdroid.db.Account as DbAccount @HiltAndroidTest class AccountsCleanupWorkerTest { @@ -49,20 +46,22 @@ class AccountsCleanupWorkerTest { @Inject lateinit var workerFactory: HiltWorkerFactory - lateinit var accountManager: AccountManager + lateinit var account: Account + lateinit var service: Service + + val accountManager by lazy { AccountManager.get(context) } lateinit var addressBookAccountType: String lateinit var addressBookAccount: Account - lateinit var service: Service @Before fun setUp() { hiltRule.inject() TestUtils.setUpWorkManager(context, workerFactory) - service = createTestService(Service.TYPE_CARDDAV) + account = TestAccountAuthenticator.create() + db.accountDao().insertOrIgnore(DbAccount(name = account.name)) // Prepare test account - accountManager = AccountManager.get(context) addressBookAccountType = context.getString(R.string.account_type_address_book) addressBookAccount = Account( "Fancy address book account", @@ -74,48 +73,8 @@ class AccountsCleanupWorkerTest { fun tearDown() { // Remove the account here in any case; Nice to have when the test fails accountManager.removeAccountExplicitly(addressBookAccount) - } - - - @Test - fun testCleanUpServices_noAccount() { - // Insert service that reference to invalid account - db.serviceDao().insertOrReplace(Service(id = 1, accountName = "test", type = Service.TYPE_CALDAV, principal = null)) - assertNotNull(db.serviceDao().get(1)) - // Create worker and run the method - val worker = TestListenableWorkerBuilder(context) - .setWorkerFactory(workerFactory) - .build() - worker.cleanUpServices() - - // Verify that service is deleted - assertNull(db.serviceDao().get(1)) - } - - @Test - fun testCleanUpServices_oneAccount() { - val account = Account("test", "test") - val accountManager = AccountManager.get(context) - mockkObject(accountManager) - every { accountManager.getAccountsByType(context.getString(R.string.account_type)) } returns arrayOf(account) - - // Insert services, one that reference the existing account and one that references an invalid account - db.serviceDao().insertOrReplace(Service(id = 1, accountName = account.name, type = Service.TYPE_CALDAV, principal = null)) - assertNotNull(db.serviceDao().get(1)) - - db.serviceDao().insertOrReplace(Service(id = 2, accountName = "not existing", type = Service.TYPE_CARDDAV, principal = null)) - assertNotNull(db.serviceDao().get(2)) - - // Create worker and run the method - val worker = TestListenableWorkerBuilder(context) - .setWorkerFactory(workerFactory) - .build() - worker.cleanUpServices() - - // Verify that one service is deleted and the other one is kept - assertNotNull(db.serviceDao().get(1)) - assertNull(db.serviceDao().get(2)) + TestAccountAuthenticator.remove(account) } @@ -161,13 +120,4 @@ class AccountsCleanupWorkerTest { } } - - // helpers - - 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)!! - } - } \ No newline at end of file diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/account/TestAccountAuthenticator.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/account/TestAccountAuthenticator.kt index d6e9b2c46..08028d42f 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/account/TestAccountAuthenticator.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/account/TestAccountAuthenticator.kt @@ -50,8 +50,7 @@ class TestAccountAuthenticator: Service() { * * Remove it with [remove]. */ - fun create(version: Int = AccountSettings.CURRENT_VERSION): Account { - val accountType = context.getString(R.string.account_type_test) + fun create(version: Int = AccountSettings.CURRENT_VERSION, accountType: String = context.getString(R.string.account_type_test)): Account { val account = Account("Test Account No. ${counter.incrementAndGet()}", accountType) val initialData = AccountSettings.initialUserData(null) @@ -72,8 +71,8 @@ class TestAccountAuthenticator: Service() { /** * Convenience method to create a test account and remove it after executing the block. */ - fun provide(version: Int = AccountSettings.CURRENT_VERSION, block: (Account) -> Unit) { - val account = create(version) + fun provide(version: Int = AccountSettings.CURRENT_VERSION, accountType: String = context.getString(R.string.account_type_test), block: (Account) -> Unit) { + val account = create(version, accountType) try { block(account) } finally { diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/AccountDao.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/AccountDao.kt index cfc9481a3..5aa43a9a1 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/db/AccountDao.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/db/AccountDao.kt @@ -8,16 +8,24 @@ import androidx.room.Dao import androidx.room.Insert import androidx.room.OnConflictStrategy import androidx.room.Query +import org.jetbrains.annotations.TestOnly @Dao interface AccountDao { + @TestOnly + @Query("SELECT * FROM account") + fun getAll(): List + @Insert(onConflict = OnConflictStrategy.IGNORE) fun insertOrIgnore(account: Account) @Query("DELETE FROM account WHERE name=:name") fun deleteByName(name: String) + @Query("DELETE FROM account WHERE name NOT IN (:names)") + fun deleteExceptNames(names: List) + @Query("UPDATE account SET name=:newName WHERE name=:oldName") fun rename(oldName: String, newName: String) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt index 647c178ad..a64a47b96 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/db/ServiceDao.kt @@ -37,7 +37,4 @@ interface ServiceDao { @Query("DELETE FROM service WHERE accountName NOT IN (:accountNames)") fun deleteExceptAccounts(accountNames: Array) - @Query("UPDATE service SET accountName=:newName WHERE accountName=:oldName") - suspend fun renameAccount(oldName: String, newName: String) - } \ No newline at end of file diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/AccountRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/AccountRepository.kt index cf291cc08..b50b3ef44 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/repository/AccountRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/repository/AccountRepository.kt @@ -205,6 +205,14 @@ class AccountRepository @Inject constructor( list }.flowOn(Dispatchers.IO) + /** + * Deletes accounts in the database which don't have a corresponding system account. + */ + fun removeOrphanedInDb() { + val accounts = accountManager.getAccountsByType(accountType) + dao.deleteExceptNames(accounts.map { it.name }) + } + /** * Renames an account. * diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt index 53b8e49a7..e3f09c096 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavServiceRepository.kt @@ -34,9 +34,6 @@ class DavServiceRepository @Inject constructor( fun insertOrReplace(service: Service) = dao.insertOrReplace(service) - suspend fun renameAccount(oldName: String, newName: String) = - dao.renameAccount(oldName, newName) - // Delete diff --git a/app/src/main/kotlin/at/bitfire/davdroid/sync/account/AccountsCleanupWorker.kt b/app/src/main/kotlin/at/bitfire/davdroid/sync/account/AccountsCleanupWorker.kt index a0b0137f9..b55d84ae8 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/sync/account/AccountsCleanupWorker.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/sync/account/AccountsCleanupWorker.kt @@ -23,7 +23,6 @@ import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject import java.time.Duration import java.util.concurrent.Semaphore -import java.util.logging.Level import java.util.logging.Logger @HiltWorker @@ -46,7 +45,7 @@ class AccountsCleanupWorker @AssistedInject constructor( override fun doWork(): Result { lockAccountsCleanup() try { - cleanUpServices() + accountRepository.removeOrphanedInDb() cleanUpAddressBooks() } finally { unlockAccountsCleanup() @@ -54,23 +53,6 @@ class AccountsCleanupWorker @AssistedInject constructor( return Result.success() } - /** - * Deletes services in the database which are not associated to a valid account. - */ - @VisibleForTesting - internal fun cleanUpServices() { - // Later, accounts which are not in the DB should be deleted here - - // Delete orphaned services in DB – only necessary as long as accounts are implemented as system accounts (not in DB) - val accounts = accountRepository.getAll() - logger.log(Level.INFO, "Cleaning up accounts. Currently existing accounts:", accounts) - val serviceDao = db.serviceDao() - if (accounts.isEmpty()) - serviceDao.deleteAll() - else - serviceDao.deleteExceptAccounts(accounts.map { it.name }.toTypedArray()) - } - /** * Deletes address book accounts which are not assigned to a valid account. */