Fix old address book accounts not being deleted

app/src/androidTest/kotlin/at/bitfire/davdroid/sync/account/AccountsCleanupWorkerTest.kt

@@ -0,0 +1,146 @@
+package at.bitfire.davdroid.sync.account
+
+import android.accounts.Account
+import android.accounts.AccountManager
+import android.content.Context
+import android.os.Bundle
+import android.util.Log
+import androidx.work.Configuration
+import androidx.work.WorkerFactory
+import androidx.work.WorkerParameters
+import androidx.work.testing.TestListenableWorkerBuilder
+import androidx.work.testing.WorkManagerTestInitHelper
+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.resource.LocalAddressBook.Companion.USER_DATA_COLLECTION_ID
+import at.bitfire.davdroid.settings.SettingsManager
+import at.bitfire.davdroid.util.setAndVerifyUserData
+import dagger.hilt.android.qualifiers.ApplicationContext
+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.Assert.assertTrue
+import org.junit.Before
+import org.junit.Rule
+import org.junit.Test
+import javax.inject.Inject
+
+@HiltAndroidTest
+class AccountsCleanupWorkerTest {
+
+    @get:Rule
+    val hiltRule = HiltAndroidRule(this)
+
+    @Inject
+    @ApplicationContext
+    lateinit var context: Context
+
+    @Inject
+    lateinit var accountsCleanupWorkerFactory: AccountsCleanupWorker.Factory
+
+    @Inject
+    lateinit var db: AppDatabase
+
+    @Inject
+    lateinit var settingsManager: SettingsManager
+
+    lateinit var accountManager: AccountManager
+    lateinit var addressBookAccountType: String
+    lateinit var addressBookAccount: Account
+    lateinit var service: Service
+
+    @Before
+    fun setUp() {
+        hiltRule.inject()
+
+        service = createTestService(Service.TYPE_CARDDAV)!!
+
+        // Create test account
+        accountManager = AccountManager.get(context)
+        addressBookAccountType = context.getString(R.string.account_type_address_book)
+        addressBookAccount = Account(
+            "Fancy address book account",
+            addressBookAccountType
+        )
+
+        // Initialize WorkManager for instrumentation tests.
+        val config = Configuration.Builder()
+            .setMinimumLoggingLevel(Log.DEBUG)
+            .build()
+        WorkManagerTestInitHelper.initializeTestWorkManager(context, config)
+    }
+
+    @After
+    fun tearDown() {
+        // Remove the account here in any case; Nice to have when the test fails
+        accountManager.removeAccountExplicitly(addressBookAccount)
+    }
+
+    @Test
+    fun testDeleteOrphanAddressBookAccounts_deletesAddressBookAccountsWithoutCollection() {
+        // Create address book account without corresponding collection
+        assertTrue(accountManager.addAccountExplicitly(addressBookAccount, null, null))
+        val addressBookAccounts = accountManager.getAccountsByType(addressBookAccountType)
+        assertEquals(addressBookAccount, addressBookAccounts.firstOrNull())
+
+        // Create worker and run the method
+        val worker = TestListenableWorkerBuilder<AccountsCleanupWorker>(context)
+            .setWorkerFactory(object: WorkerFactory() {
+                override fun createWorker(appContext: Context, workerClassName: String, workerParameters: WorkerParameters) =
+                    accountsCleanupWorkerFactory.create(appContext, workerParameters)
+            })
+            .build()
+        worker.deleteOrphanAddressBookAccounts(addressBookAccounts)
+
+        // Verify account was deleted
+        assertTrue(accountManager.getAccountsByType(addressBookAccountType).isEmpty())
+    }
+
+    @Test
+    fun testDeleteOrphanAddressBookAccounts_leavesAddressBookAccountsWithoutCollection() {
+        // Create address book account _with_ corresponding collection and verify
+        val theAccountId = 0L
+        val userData = Bundle(1).apply {
+            putString(USER_DATA_COLLECTION_ID, "$theAccountId")
+        }
+        assertTrue(accountManager.addAccountExplicitly(addressBookAccount, null, userData))
+        accountManager.setAndVerifyUserData(addressBookAccount, USER_DATA_COLLECTION_ID, "$theAccountId")
+        val addressBookAccounts = accountManager.getAccountsByType(addressBookAccountType)
+        assertEquals("$theAccountId", accountManager.getUserData(addressBookAccount, USER_DATA_COLLECTION_ID))
+
+        // Create the collection
+        val collectionDao = db.collectionDao()
+        collectionDao.insert(Collection(
+            theAccountId,
+            serviceId = service.id,
+            type = Collection.TYPE_ADDRESSBOOK,
+            url = "http://www.example.com/yay.php".toHttpUrl()
+        ))
+
+        // Create worker and run the method
+        val worker = TestListenableWorkerBuilder<AccountsCleanupWorker>(context)
+            .setWorkerFactory(object: WorkerFactory() {
+                override fun createWorker(appContext: Context, workerClassName: String, workerParameters: WorkerParameters) =
+                    accountsCleanupWorkerFactory.create(appContext, workerParameters)
+            })
+            .build()
+        worker.deleteOrphanAddressBookAccounts(addressBookAccounts)
+
+        // Verify account was _not_ deleted
+        assertEquals(addressBookAccount, addressBookAccounts.firstOrNull())
+    }
+
+
+    // 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)
+    }
+
+}

app/src/main/kotlin/at/bitfire/davdroid/App.kt

@@ -62,7 +62,7 @@ class App: Application(), Configuration.Provider {
         @OptIn(DelicateCoroutinesApi::class)
         GlobalScope.launch(Dispatchers.Default) {
             // clean up orphaned accounts in DB from time to time
-            AccountsCleanupWorker.enqueue(this@App)
+            AccountsCleanupWorker.enable(this@App)
 
             // create/update app shortcuts
             UiUtils.updateShortcuts(this@App)

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

@@ -57,6 +57,7 @@ class AccountRepository @Inject constructor(
 ) {
 
     private val accountType = context.getString(R.string.account_type)
+    private val addressBookAccountType = context.getString(R.string.account_type_address_book)
     private val accountManager = AccountManager.get(context)
 
     /**
@@ -165,8 +166,17 @@ class AccountRepository @Inject constructor(
     fun fromName(accountName: String) =
         Account(accountName, accountType)
 
+    /**
+     * Gets all real accounts (but not address book accounts)
+     */
     fun getAll(): Array<Account> = accountManager.getAccountsByType(accountType)
 
+    /**
+     * Gets only address book accounts
+     */
+    fun getAddressBookAccounts(): Array<Account> =
+        accountManager.getAccountsByType(addressBookAccountType)
+
     fun getAllFlow() = callbackFlow<Set<Account>> {
         val listener = OnAccountsUpdateListener { accounts ->
             trySend(accounts.filter { it.type == accountType }.toSet())
@@ -180,6 +190,16 @@ class AccountRepository @Inject constructor(
         }
     }
 
+    /**
+     * Get user data stored in account.
+     */
+    fun getUserData(account: Account, key: String) = accountManager.getUserData(account, key)
+
+    /**
+     * Removes the account in question directly in the background, without user interaction.
+     */
+    fun removeAccountExplicitly(account: Account) = accountManager.removeAccountExplicitly(account)
+
     /**
      * Renames an account.
      *

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

@@ -188,6 +188,8 @@ class DavCollectionRepository @Inject constructor(
 
     fun getByService(serviceId: Long) = dao.getByService(serviceId)
 
+    fun getByServiceAndUrl(serviceId: Long, url: String) = dao.getByServiceAndUrl(serviceId, url)
+
     fun getByServiceAndSync(serviceId: Long) = dao.getByServiceAndSync(serviceId)
 
     fun getSyncCalendars(serviceId: Long) = dao.getSyncCalendars(serviceId)

app/src/main/kotlin/at/bitfire/davdroid/settings/AccountSettings.kt

@@ -59,91 +59,6 @@ class AccountSettings @AssistedInject constructor(
         fun create(account: Account): AccountSettings
     }
 
-    companion object {
-
-        const val CURRENT_VERSION = 16
-        const val KEY_SETTINGS_VERSION = "version"
-
-        const val KEY_SYNC_INTERVAL_ADDRESSBOOKS = "sync_interval_addressbooks"
-        const val KEY_SYNC_INTERVAL_CALENDARS = "sync_interval_calendars"
-
-        /** Stores the tasks sync interval (in seconds) so that it can be set again when the provider is switched */
-        const val KEY_SYNC_INTERVAL_TASKS = "sync_interval_tasks"
-
-        const val KEY_USERNAME = "user_name"
-        const val KEY_CERTIFICATE_ALIAS = "certificate_alias"
-
-        /** OAuth [AuthState] (serialized as JSON) */
-        const val KEY_AUTH_STATE = "auth_state"
-
-        const val KEY_WIFI_ONLY = "wifi_only"               // sync on WiFi only (default: false)
-        const val KEY_WIFI_ONLY_SSIDS = "wifi_only_ssids"   // restrict sync to specific WiFi SSIDs
-        const val KEY_IGNORE_VPNS = "ignore_vpns"           // ignore vpns at connection detection
-
-        /** Time range limitation to the past [in days]. Values:
-         *
-         * - null: default value (DEFAULT_TIME_RANGE_PAST_DAYS)
-         * - <0 (typically -1): no limit
-         * - n>0: entries more than n days in the past won't be synchronized
-         */
-        const val KEY_TIME_RANGE_PAST_DAYS = "time_range_past_days"
-        const val DEFAULT_TIME_RANGE_PAST_DAYS = 90
-
-        /**
-         * Whether a default alarm shall be assigned to received events/tasks which don't have an alarm.
-         * Value can be null (no default alarm) or an integer (default alarm shall be created this
-         * number of minutes before the event/task).
-         */
-        const val KEY_DEFAULT_ALARM = "default_alarm"
-
-        /** Whether DAVx5 sets the local calendar color to the value from service DB at every sync
-         value = *null* (not existing): true (default);
-                 "0"                    false */
-        const val KEY_MANAGE_CALENDAR_COLORS = "manage_calendar_colors"
-
-        /** Whether DAVx5 populates and uses CalendarContract.Colors
-         value = *null* (not existing)   false (default);
-                 "1"                     true */
-        const val KEY_EVENT_COLORS = "event_colors"
-
-        /** Contact group method:
-         *null (not existing)*     groups as separate vCards (default);
-         "CATEGORIES"              groups are per-contact CATEGORIES
-         */
-        const val KEY_CONTACT_GROUP_METHOD = "contact_group_method"
-
-        /** UI preference: Show only personal collections
-         value = *null* (not existing)   show all collections (default);
-         "1"                             show only personal collections */
-        const val KEY_SHOW_ONLY_PERSONAL = "show_only_personal"
-
-        const val SYNC_INTERVAL_MANUALLY = -1L
-
-        /** Static property to indicate whether AccountSettings migration is currently running.
-         * **Access must be `synchronized` with `AccountSettings::class.java`.** */
-        @Volatile
-        var currentlyUpdating = false
-
-        fun initialUserData(credentials: Credentials?): Bundle {
-            val bundle = Bundle()
-            bundle.putString(KEY_SETTINGS_VERSION, CURRENT_VERSION.toString())
-
-            if (credentials != null) {
-                if (credentials.username != null)
-                    bundle.putString(KEY_USERNAME, credentials.username)
-
-                if (credentials.certificateAlias != null)
-                    bundle.putString(KEY_CERTIFICATE_ALIAS, credentials.certificateAlias)
-
-                if (credentials.authState != null)
-                    bundle.putString(KEY_AUTH_STATE, credentials.authState.jsonSerializeString())
-            }
-
-            return bundle
-        }
-
-    }
-
     init {
         if (Looper.getMainLooper() == Looper.myLooper())
             throw IllegalThreadStateException("AccountSettings may not be used on main thread")
@@ -520,4 +435,89 @@ class AccountSettings @AssistedInject constructor(
         }
     }
 
+    companion object {
+
+        const val CURRENT_VERSION = 17
+        const val KEY_SETTINGS_VERSION = "version"
+
+        const val KEY_SYNC_INTERVAL_ADDRESSBOOKS = "sync_interval_addressbooks"
+        const val KEY_SYNC_INTERVAL_CALENDARS = "sync_interval_calendars"
+
+        /** Stores the tasks sync interval (in seconds) so that it can be set again when the provider is switched */
+        const val KEY_SYNC_INTERVAL_TASKS = "sync_interval_tasks"
+
+        const val KEY_USERNAME = "user_name"
+        const val KEY_CERTIFICATE_ALIAS = "certificate_alias"
+
+        /** OAuth [AuthState] (serialized as JSON) */
+        const val KEY_AUTH_STATE = "auth_state"
+
+        const val KEY_WIFI_ONLY = "wifi_only"               // sync on WiFi only (default: false)
+        const val KEY_WIFI_ONLY_SSIDS = "wifi_only_ssids"   // restrict sync to specific WiFi SSIDs
+        const val KEY_IGNORE_VPNS = "ignore_vpns"           // ignore vpns at connection detection
+
+        /** Time range limitation to the past [in days]. Values:
+         *
+         * - null: default value (DEFAULT_TIME_RANGE_PAST_DAYS)
+         * - <0 (typically -1): no limit
+         * - n>0: entries more than n days in the past won't be synchronized
+         */
+        const val KEY_TIME_RANGE_PAST_DAYS = "time_range_past_days"
+        const val DEFAULT_TIME_RANGE_PAST_DAYS = 90
+
+        /**
+         * Whether a default alarm shall be assigned to received events/tasks which don't have an alarm.
+         * Value can be null (no default alarm) or an integer (default alarm shall be created this
+         * number of minutes before the event/task).
+         */
+        const val KEY_DEFAULT_ALARM = "default_alarm"
+
+        /** Whether DAVx5 sets the local calendar color to the value from service DB at every sync
+        value = *null* (not existing): true (default);
+        "0"                    false */
+        const val KEY_MANAGE_CALENDAR_COLORS = "manage_calendar_colors"
+
+        /** Whether DAVx5 populates and uses CalendarContract.Colors
+        value = *null* (not existing)   false (default);
+        "1"                     true */
+        const val KEY_EVENT_COLORS = "event_colors"
+
+        /** Contact group method:
+         *null (not existing)*     groups as separate vCards (default);
+        "CATEGORIES"              groups are per-contact CATEGORIES
+         */
+        const val KEY_CONTACT_GROUP_METHOD = "contact_group_method"
+
+        /** UI preference: Show only personal collections
+        value = *null* (not existing)   show all collections (default);
+        "1"                             show only personal collections */
+        const val KEY_SHOW_ONLY_PERSONAL = "show_only_personal"
+
+        const val SYNC_INTERVAL_MANUALLY = -1L
+
+        /** Static property to indicate whether AccountSettings migration is currently running.
+         * **Access must be `synchronized` with `AccountSettings::class.java`.** */
+        @Volatile
+        var currentlyUpdating = false
+
+        fun initialUserData(credentials: Credentials?): Bundle {
+            val bundle = Bundle()
+            bundle.putString(KEY_SETTINGS_VERSION, CURRENT_VERSION.toString())
+
+            if (credentials != null) {
+                if (credentials.username != null)
+                    bundle.putString(KEY_USERNAME, credentials.username)
+
+                if (credentials.certificateAlias != null)
+                    bundle.putString(KEY_CERTIFICATE_ALIAS, credentials.certificateAlias)
+
+                if (credentials.authState != null)
+                    bundle.putString(KEY_AUTH_STATE, credentials.authState.jsonSerializeString())
+            }
+
+            return bundle
+        }
+
+    }
+
 }