Skip to content

Commit

Permalink
Set or update read-only flag when address books are renamed (#1124)
Browse files Browse the repository at this point in the history
* Always evaluate read only

* Extract read only evaluation to companion method

* Extract read only evaluation to companion method

* Add test

* Always pass forceReadOnly flag

* Minor KDoc changes

---------

Co-authored-by: Ricki Hirner <[email protected]>
  • Loading branch information
sunkup and rfc2822 authored Nov 7, 2024
1 parent cf15dd3 commit 32925dc
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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()
}
Expand Down Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}
}
}
Expand Down

0 comments on commit 32925dc

Please sign in to comment.