Set or update read-only flag when address books are renamed

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

@@ -12,20 +12,24 @@ import android.content.Context
 import android.provider.ContactsContract
 import androidx.test.platform.app.InstrumentationRegistry
 import androidx.test.rule.GrantPermissionRule
+import at.bitfire.davdroid.db.Collection
 import at.bitfire.vcard4android.Contact
 import at.bitfire.vcard4android.GroupMethod
 import at.bitfire.vcard4android.LabeledProperty
 import dagger.hilt.android.qualifiers.ApplicationContext
 import dagger.hilt.android.testing.HiltAndroidRule
 import dagger.hilt.android.testing.HiltAndroidTest
 import ezvcard.property.Telephone
+import io.mockk.every
+import io.mockk.mockk
 import java.util.LinkedList
 import javax.inject.Inject
 import org.junit.After
 import org.junit.AfterClass
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.BeforeClass
 import org.junit.ClassRule
@@ -123,6 +127,19 @@ class LocalAddressBookTest {
         assertEquals("Test Group", group.displayName)
     }
 
+    /**
+     * Tests the calculation of read only state is correct
+     */
+    @Test
+    fun test_shouldBeReadOnly() {
+        val collectionReadOnly = mockk<Collection> { every { readOnly() } returns true }
+        assertTrue(LocalAddressBook.shouldBeReadOnly(collectionReadOnly, false))
+        assertTrue(LocalAddressBook.shouldBeReadOnly(collectionReadOnly, true))
+
+        val collectionNotReadOnly = mockk<Collection> { every { readOnly() } returns false }
+        assertFalse(LocalAddressBook.shouldBeReadOnly(collectionNotReadOnly, false))
+        assertTrue(LocalAddressBook.shouldBeReadOnly(collectionNotReadOnly, true))
+    }
 
 
     companion object {

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

@@ -155,9 +155,8 @@ open class LocalAddressBook @AssistedInject constructor(
      * @param info  collection where to take the settings from
      * @param forceReadOnly  `true`: set the address book to "force read-only";
      *                       `false`: determine read-only flag from [info];
-     *                       `null`: don't change the existing value
      */
-    fun update(info: Collection, forceReadOnly: Boolean? = null) {
+    fun update(info: Collection, forceReadOnly: Boolean) {
         logger.log(Level.INFO, "Updating local address book $addressBookAccount with collection $info")
         val accountManager = AccountManager.get(context)
 
@@ -175,31 +174,30 @@ open class LocalAddressBook @AssistedInject constructor(
         settings = contactsProviderSettings
 
         // Update force read only
-        if (forceReadOnly != null) {
-            val nowReadOnly = forceReadOnly || !info.privWriteContent || info.forceReadOnly
-            if (nowReadOnly != readOnly) {
-                logger.info("Address book now read-only = $nowReadOnly, updating contacts")
-
-                // update address book itself
-                readOnly = nowReadOnly
-
-                // update raw contacts
-                val rawContactValues = ContentValues(1)
-                rawContactValues.put(RawContacts.RAW_CONTACT_IS_READ_ONLY, if (nowReadOnly) 1 else 0)
-                provider!!.update(rawContactsSyncUri(), rawContactValues, null, null)
-
-                // update data rows
-                val dataValues = ContentValues(1)
-                dataValues.put(ContactsContract.Data.IS_READ_ONLY, if (nowReadOnly) 1 else 0)
-                provider!!.update(syncAdapterURI(ContactsContract.Data.CONTENT_URI), dataValues, null, null)
-
-                // update group rows
-                val groupValues = ContentValues(1)
-                groupValues.put(Groups.GROUP_IS_READ_ONLY, if (nowReadOnly) 1 else 0)
-                provider!!.update(groupsSyncUri(), groupValues, null, null)
-            }
+        val nowReadOnly = shouldBeReadOnly(info, forceReadOnly)
+        if (nowReadOnly != readOnly) {
+            logger.info("Address book now read-only = $nowReadOnly, updating contacts")
+
+            // update address book itself
+            readOnly = nowReadOnly
+
+            // update raw contacts
+            val rawContactValues = ContentValues(1)
+            rawContactValues.put(RawContacts.RAW_CONTACT_IS_READ_ONLY, if (nowReadOnly) 1 else 0)
+            provider!!.update(rawContactsSyncUri(), rawContactValues, null, null)
+
+            // update data rows
+            val dataValues = ContentValues(1)
+            dataValues.put(ContactsContract.Data.IS_READ_ONLY, if (nowReadOnly) 1 else 0)
+            provider!!.update(syncAdapterURI(ContactsContract.Data.CONTENT_URI), dataValues, null, null)
+
+            // update group rows
+            val groupValues = ContentValues(1)
+            groupValues.put(Groups.GROUP_IS_READ_ONLY, if (nowReadOnly) 1 else 0)
+            provider!!.update(groupsSyncUri(), groupValues, null, null)
         }
 
+
         // make sure it will still be synchronized when contacts are updated
         updateSyncFrameworkSettings()
     }
@@ -432,11 +430,21 @@ open class LocalAddressBook @AssistedInject constructor(
 
             addressBook.updateSyncFrameworkSettings()
             addressBook.settings = contactsProviderSettings
-            addressBook.readOnly = forceReadOnly || !info.privWriteContent || info.forceReadOnly
+            addressBook.readOnly = shouldBeReadOnly(info, forceReadOnly)
 
             return addressBook
         }
 
+        /**
+         * Determines whether the address book should be set to read-only.
+         *
+         * @param forceReadOnly     Whether (usually managed, app-wide) setting should overwrite local read-only information
+         * @param info              Collection data to determine read-only status from (either user-set read-only flag or missing write privilege)
+         */
+        @VisibleForTesting
+        internal fun shouldBeReadOnly(info: Collection, forceReadOnly: Boolean): Boolean =
+            info.readOnly() || forceReadOnly
+
         /**
          * Finds a [LocalAddressBook] based on its corresponding collection.
          *

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

@@ -21,7 +21,6 @@ import androidx.work.WorkManager
 import at.bitfire.davdroid.R
 import at.bitfire.davdroid.db.AppDatabase
 import at.bitfire.davdroid.db.Service
-import at.bitfire.davdroid.repository.AccountRepository
 import at.bitfire.davdroid.repository.DavCollectionRepository
 import at.bitfire.davdroid.repository.DavServiceRepository
 import at.bitfire.davdroid.resource.LocalAddressBook
@@ -52,7 +51,6 @@ class AccountSettingsMigrations @AssistedInject constructor(
     @Assisted val account: Account,
     @Assisted val accountSettings: AccountSettings,
     @ApplicationContext val context: Context,
-    private val accountRepository: AccountRepository,
     private val collectionRepository: DavCollectionRepository,
     private val db: AppDatabase,
     private val localAddressBookFactory: LocalAddressBook.Factory,
@@ -103,7 +101,7 @@ class AccountSettingsMigrations @AssistedInject constructor(
                 collectionRepository.getByServiceAndUrl(service.id, url)?.let { collection ->
                     // Set collection ID and rename the account
                     val localAddressBook = localAddressBookFactory.create(oldAddressBookAccount, provider)
-                    localAddressBook.update(collection)
+                    localAddressBook.update(collection, /* read-only flag will be updated at next sync */ forceReadOnly = false)
                 }
             }
         }