Don't make contacts dirty when moving

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

@@ -7,6 +7,7 @@ package at.bitfire.davdroid.resource
 import android.Manifest
 import android.accounts.Account
 import android.content.ContentProviderClient
+import android.content.ContentUris
 import android.content.Context
 import android.provider.ContactsContract
 import androidx.test.platform.app.InstrumentationRegistry
@@ -20,6 +21,7 @@ import dagger.hilt.android.testing.HiltAndroidTest
 import ezvcard.property.Telephone
 import org.junit.AfterClass
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNotNull
 import org.junit.Before
 import org.junit.BeforeClass
@@ -64,18 +66,25 @@ class LocalAddressBookTest {
                 displayName = "Test Contact",
                 phoneNumbers = LinkedList(listOf(LabeledProperty(Telephone("1234567890"))))
             )
-            LocalContact(addressBook, contact, null, null, 0).add()
+            val uri = LocalContact(addressBook, contact, null, null, 0).add()
+            val id = ContentUris.parseId(uri)
+            val localContact = addressBook.findContactById(id)
+            localContact.resetDirty()
+            assertFalse("Contact is dirty before moving", localContact.isDirty())
 
             // rename address book
-            val newAccount = Account("New Name", addressBook.account.type)
-            LocalAddressBook.renameAccount(context, provider, addressBook.account, newAccount.name)
-            addressBook.account = newAccount
-
-            // check whether contact is still there
-            val result = addressBook.findContactByUid(uid)!!.getContact()
-            assertEquals(uid, result.uid)
-            assertEquals(contact.displayName, result.displayName)
-            assertEquals(contact.phoneNumbers.first().component1().text, result.phoneNumbers.first().component1().text)
+            val newName = "New Name"
+            addressBook.renameAccount(newName)
+            assertEquals(Account(newName, LocalTestAddressBook.ACCOUNT.type), addressBook.account)
+
+            // check whether contact is still here (including data rows) and not dirty
+            val result = addressBook.findContactById(id)
+            assertFalse("Contact is dirty after moving", result.isDirty())
+
+            val contact2 = result.getContact()
+            assertEquals(uid, contact2.uid)
+            assertEquals(contact.displayName, contact2.displayName)
+            assertEquals(contact.phoneNumbers.first().component1().text, contact2.phoneNumbers.first().component1().text)
 
         } finally {
             // clean up / remove address book

app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBook.kt

@@ -73,7 +73,8 @@ open class LocalAddressBook @AssistedInject constructor(
     override val tag: String
         get() = "contacts-${account.name}"
 
-    override val title = addressBookAccount.name
+    override val title
+        get() = account.name
 
     /**
      * Whether contact groups ([LocalGroup]) are included in query results
@@ -84,16 +85,16 @@ open class LocalAddressBook @AssistedInject constructor(
      */
     open val groupMethod: GroupMethod by lazy {
         val manager = AccountManager.get(context)
-        val account = manager.getUserData(addressBookAccount, USER_DATA_COLLECTION_ID)?.toLongOrNull()?.let { collectionId ->
+        val associatedAccount = manager.getUserData(/* address book account */ account, USER_DATA_COLLECTION_ID)?.toLongOrNull()?.let { collectionId ->
             collectionRepository.get(collectionId)?.let { collection ->
                 serviceRepository.get(collection.serviceId)?.let { service ->
                     Account(service.accountName, context.getString(R.string.account_type))
                 }
             }
         }
-        if (account == null)
+        if (associatedAccount == null)
             throw IllegalArgumentException("Collection of address book account $addressBookAccount does not have an account")
-        val accountSettings = accountSettingsFactory.create(account)
+        val accountSettings = accountSettingsFactory.create(associatedAccount)
         accountSettings.getGroupMethod()
     }
     private val includeGroups
@@ -158,7 +159,8 @@ open class LocalAddressBook @AssistedInject constructor(
         // Update the account name
         val newAccountName = accountName(context, info)
         if (account.name != newAccountName)
-            account = renameAccount(context, provider!!, account, newAccountName)
+            // rename and update AndroidAddressBook.account
+            renameAccount(newAccountName)
 
         // Update the account user data
         accountManager.setAndVerifyUserData(account, USER_DATA_COLLECTION_ID, info.id.toString())
@@ -194,6 +196,58 @@ open class LocalAddressBook @AssistedInject constructor(
         updateSyncFrameworkSettings()
     }
 
+    /**
+     * Renames an address book account and moves the contacts.
+     *
+     * On success, [account] will be updated to the new account name.
+     *
+     * _Note:_ Previously, we had used [AccountManager.renameAccount], but then the contacts can't be moved because there's never
+     * a moment when both accounts are available.
+     *
+     * @param newName   the new account name (will have same account type as [account])
+     *
+     * @return whether the account was renamed successfully
+     */
+    @VisibleForTesting
+    internal fun renameAccount(newName: String): Boolean {
+        val oldAccount = account
+        logger.info("Renaming address book from \"${oldAccount.name}\" to \"$newName\"")
+
+        // copy user data to new account
+        val accountManager = AccountManager.get(context)
+        val userData = Bundle(2).apply {
+            putString(USER_DATA_COLLECTION_ID, accountManager.getUserData(oldAccount, USER_DATA_COLLECTION_ID))
+            putString(USER_DATA_URL, accountManager.getUserData(oldAccount, USER_DATA_URL))
+        }
+
+        val newAccount = Account(newName, oldAccount.type)
+        if (!SystemAccountUtils.createAccount(context, newAccount, userData))
+            // Couldn't rename account
+            return false
+
+        // move contacts and groups to new account
+        val batch = BatchOperation(provider!!)
+        batch.enqueue(BatchOperation.CpoBuilder
+            .newUpdate(groupsSyncUri())
+            .withSelection(Groups.ACCOUNT_NAME + "=? AND " + Groups.ACCOUNT_TYPE + "=?", arrayOf(oldAccount.name, oldAccount.type))
+            .withValue(Groups.ACCOUNT_NAME, newAccount.name)
+        )
+        batch.enqueue(BatchOperation.CpoBuilder
+            .newUpdate(rawContactsSyncUri())
+            .withSelection(RawContacts.ACCOUNT_NAME + "=? AND " + RawContacts.ACCOUNT_TYPE + "=?", arrayOf(oldAccount.name, oldAccount.type))
+            .withValue(RawContacts.ACCOUNT_NAME, newAccount.name)
+        )
+        batch.commit()
+
+        // update AndroidAddressBook.account
+        account = newAccount
+
+        // delete old account
+        accountManager.removeAccountExplicitly(oldAccount)
+
+        return true
+    }
+
     override fun deleteCollection(): Boolean {
         val accountManager = AccountManager.get(context)
         return accountManager.removeAccountExplicitly(account)
@@ -479,51 +533,6 @@ open class LocalAddressBook @AssistedInject constructor(
             return bundle
         }
 
-        /**
-         * Renames an address book account and moves the contacts.
-         *
-         * @param provider    used to move the contacts
-         * @param oldAccount  the account to rename
-         * @param newName     the new account name (will have same account type)
-         *
-         * Previously, we had used [AccountManager.renameAccount], but then the contacts can't be moved because there's never
-         * a moment when both accounts are available.
-         *
-         * @return the resulting account name (new name if successful, old name otherwise)
-         */
-        @VisibleForTesting
-        internal fun renameAccount(context: Context, provider: ContentProviderClient, oldAccount: Account, newName: String): Account {
-            val newAccount = Account(newName, oldAccount.type)
-
-            //val future = accountManager.renameAccount(oldAccount, newName, null, null)
-            val accountManager = AccountManager.get(context)
-
-            // copy user data to new account
-            val data = Bundle(2).apply {
-                putString(USER_DATA_COLLECTION_ID, accountManager.getUserData(oldAccount, USER_DATA_COLLECTION_ID))
-                putString(USER_DATA_URL, accountManager.getUserData(oldAccount, USER_DATA_URL))
-            }
-
-            if (!SystemAccountUtils.createAccount(context, newAccount, data))
-                // Couldn't rename account
-                return oldAccount
-
-            // move contacts to new account
-            val batch = BatchOperation(provider)
-            batch.enqueue(BatchOperation.CpoBuilder
-                .newUpdate(RawContacts.CONTENT_URI)
-                .withSelection(RawContacts.ACCOUNT_NAME + "=? AND " + RawContacts.ACCOUNT_TYPE + "=?", arrayOf(oldAccount.name, oldAccount.type))
-                .withValue(RawContacts.ACCOUNT_NAME, newAccount.name)
-                .withValue(RawContacts.ACCOUNT_TYPE, newAccount.type)
-            )
-            batch.commit()
-
-            // delete old account
-            accountManager.removeAccountExplicitly(oldAccount)
-
-            return newAccount
-        }
-
     }
 
 }

app/src/main/kotlin/at/bitfire/davdroid/resource/LocalContact.kt

@@ -115,6 +115,21 @@ class LocalContact: AndroidContact, LocalAddress {
         this.eTag = eTag
     }
 
+    /**
+     * Returns the dirty flag of the current contact.
+     *
+     * @return true if the contact is dirty, false otherwise
+     *
+     * @throws FileNotFoundException if the current contact can't be found
+     */
+    fun isDirty(): Boolean {
+        addressBook.provider!!.query(rawContactSyncURI(), arrayOf(ContactsContract.RawContacts.DIRTY), null, null, null)?.use { cursor ->
+            if (cursor.moveToFirst())
+                return cursor.getInt(0) != 0
+        }
+        throw FileNotFoundException()
+    }
+
     override fun resetDeleted() {
         val values = ContentValues(1)
         values.put(ContactsContract.Groups.DELETED, 0)

app/src/main/kotlin/at/bitfire/davdroid/sync/AddressBookSyncer.kt

@@ -74,7 +74,7 @@ class AddressBookSyncer @AssistedInject constructor(
     }
 
     override fun syncCollection(provider: ContentProviderClient, localCollection: LocalAddressBook, remoteCollection: Collection) {
-        logger.info("Synchronizing address book $localCollection")
+        logger.info("Synchronizing address book: ${localCollection.account.name}")
         syncAddressBook(
             account = account,
             addressBook = localCollection,
@@ -124,8 +124,6 @@ class AddressBookSyncer @AssistedInject constructor(
             }
             accountSettings.accountManager.setAndVerifyUserData(addressBook.account, PREVIOUS_GROUP_METHOD, groupMethod)
 
-            logger.info("Synchronizing address book: ${addressBook.collectionUrl}")
-
             val syncManager = contactsSyncManagerFactory.contactsSyncManager(account, accountSettings, httpClient.value, extras, authority, syncResult, provider, addressBook, collection)
             syncManager.performSync()