Skip to content

Commit

Permalink
Move dirty verifier logic to ContactDirtyVerifier (#1122)
Browse files Browse the repository at this point in the history 
* Move dirty verifier logic to ContactDirtyVerifier

* KDoc

* KDoc, fix contactDataHashCode

* Fix tests

* Add KDoc
  • Loading branch information
@rfc2822
rfc2822 authored Nov 6, 2024
1 parent 154d1e6 commit 3317a8d
Show file tree
Hide file tree
Showing 8 changed files with 262 additions and 111 deletions.
12 changes: 11 additions & 1 deletion app/src/androidTest/kotlin/at/bitfire/davdroid/resource/LocalTestAddressBook.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import dagger.assisted.AssistedInject
import dagger.hilt.android.qualifiers.ApplicationContext
import org.junit.Assert.assertTrue
import java.io.FileNotFoundException
import java.util.Optional
import java.util.logging.Logger

class LocalTestAddressBook @AssistedInject constructor(
Expand All @@ -30,7 +31,16 @@ class LocalTestAddressBook @AssistedInject constructor(
@ApplicationContext context: Context,
logger: Logger,
serviceRepository: DavServiceRepository
): LocalAddressBook(ACCOUNT, provider, accountSettingsFactory, collectionRepository, context, logger, serviceRepository) {
): LocalAddressBook(
_addressBookAccount = ACCOUNT,
provider = provider,
accountSettingsFactory = accountSettingsFactory,
collectionRepository = collectionRepository,
context = context,
dirtyVerifier = Optional.empty(),
logger = logger,
serviceRepository = serviceRepository
) {

@AssistedFactory
interface Factory {
Expand Down
39 changes: 4 additions & 35 deletions app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBook.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import android.content.ContentResolver
import android.content.ContentUris
import android.content.ContentValues
import android.content.Context
import android.os.Build
import android.os.Bundle
import android.os.RemoteException
import android.provider.ContactsContract
Expand All @@ -24,6 +23,7 @@ import at.bitfire.davdroid.db.Collection
import at.bitfire.davdroid.db.SyncState
import at.bitfire.davdroid.repository.DavCollectionRepository
import at.bitfire.davdroid.repository.DavServiceRepository
import at.bitfire.davdroid.resource.workaround.ContactDirtyVerifier
import at.bitfire.davdroid.settings.AccountSettings
import at.bitfire.davdroid.sync.account.SystemAccountUtils
import at.bitfire.davdroid.util.DavUtils.lastSegment
Expand All @@ -42,6 +42,7 @@ import dagger.hilt.android.EntryPointAccessors
import dagger.hilt.android.qualifiers.ApplicationContext
import dagger.hilt.components.SingletonComponent
import java.util.LinkedList
import java.util.Optional
import java.util.logging.Level
import java.util.logging.Logger

Expand All @@ -63,6 +64,7 @@ open class LocalAddressBook @AssistedInject constructor(
private val accountSettingsFactory: AccountSettings.Factory,
private val collectionRepository: DavCollectionRepository,
@ApplicationContext val context: Context,
val dirtyVerifier: Optional<ContactDirtyVerifier>,
private val logger: Logger,
private val serviceRepository: DavServiceRepository
): AndroidAddressBook<LocalContact, LocalGroup>(_addressBookAccount, provider, LocalContact.Factory, LocalGroup.Factory), LocalCollection<LocalAddress> {
Expand Down Expand Up @@ -100,7 +102,7 @@ open class LocalAddressBook @AssistedInject constructor(
val accountSettings = accountSettingsFactory.create(account)
accountSettings.getGroupMethod()
}
private val includeGroups
val includeGroups
get() = groupMethod == GroupMethod.GROUP_VCARDS

@Deprecated("Local collection should be identified by ID, not by URL")
Expand Down Expand Up @@ -344,39 +346,6 @@ open class LocalAddressBook @AssistedInject constructor(
}


/**
* Queries all contacts with DIRTY flag and checks whether their data checksum has changed, i.e.
* if they're "really dirty" (= data has changed, not only metadata, which is not hashed).
* The DIRTY flag is removed from contacts which are not "really dirty", i.e. from contacts
* whose contact data checksum has not changed.
* @return number of "really dirty" contacts
* @throws RemoteException on content provider errors
*/
fun verifyDirty(): Int {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O)
throw IllegalStateException("verifyDirty() should not be called on Android != 7.0")

var reallyDirty = 0
for (contact in findDirtyContacts()) {
val lastHash = contact.getLastHashCode()
val currentHash = contact.dataHashCode()
if (lastHash == currentHash) {
// hash is code still the same, contact is not "really dirty" (only metadata been have changed)
logger.log(Level.FINE, "Contact data hash has not changed, resetting dirty flag", contact)
contact.resetDirty()
} else {
logger.log(Level.FINE, "Contact data has changed from hash $lastHash to $currentHash", contact)
reallyDirty++
}
}

if (includeGroups)
reallyDirty += findDirtyGroups().size

return reallyDirty
}


/* special group operations */

/**
Expand Down
71 changes: 12 additions & 59 deletions app/src/main/kotlin/at/bitfire/davdroid/resource/LocalContact.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
package at.bitfire.davdroid.resource

import android.content.ContentValues
import android.os.Build
import android.os.RemoteException
import android.provider.ContactsContract
import android.provider.ContactsContract.CommonDataKinds.GroupMembership
Expand All @@ -25,7 +24,7 @@ import at.bitfire.vcard4android.Contact
import ezvcard.Ezvcard
import java.io.FileNotFoundException
import java.util.UUID
import java.util.logging.Logger
import kotlin.jvm.optionals.getOrNull

class LocalContact: AndroidContact, LocalAddress {

Expand All @@ -39,8 +38,6 @@ class LocalContact: AndroidContact, LocalAddress {
const val COLUMN_HASHCODE = ContactsContract.RawContacts.SYNC3
}

private val logger: Logger = Logger.getGlobal()

override val addressBook: LocalAddressBook
get() = super.addressBook as LocalAddressBook

Expand Down Expand Up @@ -91,6 +88,13 @@ class LocalContact: AndroidContact, LocalAddress {
return "$uid.vcf"
}

/**
* Clears cached [contact] so that the next read of [contact] will query the content provider again.
*/
fun clearCachedContact() {
_contact = null
}

override fun clearDirty(fileName: String?, eTag: String?, scheduleTag: String?) {
if (scheduleTag != null)
throw IllegalArgumentException("Contacts must not have a Schedule-Tag")
Expand All @@ -101,12 +105,8 @@ class LocalContact: AndroidContact, LocalAddress {
values.put(COLUMN_ETAG, eTag)
values.put(ContactsContract.RawContacts.DIRTY, 0)

if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) {
// workaround for Android 7 which sets DIRTY flag when only meta-data is changed
val hashCode = dataHashCode()
values.put(COLUMN_HASHCODE, hashCode)
logger.finer("Clearing dirty flag with eTag = $eTag, contact hash = $hashCode")
}
// Android 7 workaround
addressBook.dirtyVerifier.getOrNull()?.setHashCodeColumn(this, values)

addressBook.provider!!.update(rawContactSyncURI(), values, null, null)

Expand Down Expand Up @@ -136,54 +136,6 @@ class LocalContact: AndroidContact, LocalAddress {
}


/**
* Calculates a hash code from the contact's data (VCard) and group memberships.
* Attention: re-reads {@link #contact} from the database, discarding all changes in memory
* @return hash code of contact data (including group memberships)
*/
internal fun dataHashCode(): Int {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O)
throw IllegalStateException("dataHashCode() should not be called on Android != 7")

// reset contact so that getContact() reads from database
_contact = null

// groupMemberships is filled by getContact()
val dataHash = getContact().hashCode()
val groupHash = groupMemberships.hashCode()
logger.finest("Calculated data hash = $dataHash, group memberships hash = $groupHash")
return dataHash xor groupHash
}

fun updateHashCode(batch: BatchOperation?) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O)
throw IllegalStateException("updateHashCode() should not be called on Android != 7")

val hashCode = dataHashCode()
logger.fine("Storing contact hash = $hashCode")

if (batch == null) {
val values = ContentValues(1)
values.put(COLUMN_HASHCODE, hashCode)
addressBook.provider!!.update(rawContactSyncURI(), values, null, null)
} else
batch.enqueue(BatchOperation.CpoBuilder
.newUpdate(rawContactSyncURI())
.withValue(COLUMN_HASHCODE, hashCode))
}

fun getLastHashCode(): Int {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O)
throw IllegalStateException("getLastHashCode() should not be called on Android != 7")

addressBook.provider!!.query(rawContactSyncURI(), arrayOf(COLUMN_HASHCODE), null, null, null)?.use { c ->
if (c.moveToNext() && !c.isNull(0))
return c.getInt(0)
}
return 0
}


fun addToGroup(batch: BatchOperation, groupID: Long) {
batch.enqueue(BatchOperation.CpoBuilder
.newInsert(dataSyncURI())
Expand Down Expand Up @@ -238,6 +190,7 @@ class LocalContact: AndroidContact, LocalAddress {


// data rows

override fun buildContact(builder: BatchOperation.CpoBuilder, update: Boolean) {
builder.withValue(COLUMN_FLAGS, flags)
super.buildContact(builder, update)
Expand All @@ -250,4 +203,4 @@ class LocalContact: AndroidContact, LocalAddress {
LocalContact(addressBook as LocalAddressBook, values)
}

}
}
10 changes: 7 additions & 3 deletions app/src/main/kotlin/at/bitfire/davdroid/resource/LocalGroup.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import at.bitfire.vcard4android.Contact
import java.util.LinkedList
import java.util.UUID
import java.util.logging.Logger
import kotlin.jvm.optionals.getOrNull

class LocalGroup: AndroidGroup, LocalAddress {

Expand Down Expand Up @@ -90,11 +91,14 @@ class LocalGroup: AndroidGroup, LocalAddress {
changeContactIDs += missingMember.id!!
}

if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O)
addressBook.dirtyVerifier.getOrNull()?.let { verifier ->
// workaround for Android 7 which sets DIRTY flag when only meta-data is changed
changeContactIDs
.map { addressBook.findContactById(it) }
.forEach { it.updateHashCode(batch) }
.map { id -> addressBook.findContactById(id) }
.forEach { contact ->
verifier.updateHashCode(contact, batch)
}
}

batch.commit()
}
Expand Down
Loading

0 comments on commit 3317a8d

Please sign in to comment.