From d707a1e6433ade3f72d70caac1b1d86ae08807b2 Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Wed, 1 Feb 2017 19:18:50 +0100 Subject: [PATCH] Implement checksum to check whether DIRTY contacts have "really" changed * contact data hash code = hash code of data fields and group memberships * Before every contact sync, all dirty contacts are checked whether they're "really dirty" (= data hash code has changed). If they're not, the DIRTY flag is reset. Works around Android 7 behavior of setting contacts to DIRTY even if onky meta data has been updated (for instance, lastContacted after a call or SMS), * When an "upload" sync is initiated by notifyChange and there are no "really dirty" contacts, the sync is ignored. * contact upload: clearDirty() saves hash code, too * contact download: create()/update() saves hash code, too * debugging: sync flags (extras) are now logged --- .../davdroid/resource/LocalAddressBook.java | 28 +++++++ .../davdroid/resource/LocalContact.java | 73 ++++++++++++++++++- .../syncadapter/ContactsSyncManager.java | 7 ++ .../syncadapter/SyncAdapterService.java | 2 +- .../davdroid/ui/AccountSettingsActivity.java | 3 - 5 files changed, 106 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/at/bitfire/davdroid/resource/LocalAddressBook.java b/app/src/main/java/at/bitfire/davdroid/resource/LocalAddressBook.java index f503bfe3..1fdb0926 100644 --- a/app/src/main/java/at/bitfire/davdroid/resource/LocalAddressBook.java +++ b/app/src/main/java/at/bitfire/davdroid/resource/LocalAddressBook.java @@ -84,6 +84,34 @@ public class LocalAddressBook extends AndroidAddressBook implements LocalCollect return deleted.toArray(new LocalResource[deleted.size()]); } + /** + * 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 + */ + public int verifyDirty() throws ContactsStorageException { + int reallyDirty = 0; + for (LocalContact contact : getDirtyContacts()) { + try { + if (contact.getLastHashCode() == contact.dataHashCode()) { + // hash is code still the same, contact is not "really dirty" (only metadata been have changed) + App.log.log(Level.FINE, "Contact data hash has not changed, resetting dirty flag", contact); + contact.resetDirty(); + } else + reallyDirty++; + } catch(FileNotFoundException e) { + throw new ContactsStorageException("Couldn't calculate hash code", e); + } + } + + if (includeGroups) + reallyDirty += getDirtyGroups().length; + + return reallyDirty; + } + /** * Returns an array of local contacts/groups which have been changed locally (DIRTY != 0). */ diff --git a/app/src/main/java/at/bitfire/davdroid/resource/LocalContact.java b/app/src/main/java/at/bitfire/davdroid/resource/LocalContact.java index 74bab556..7a399b08 100644 --- a/app/src/main/java/at/bitfire/davdroid/resource/LocalContact.java +++ b/app/src/main/java/at/bitfire/davdroid/resource/LocalContact.java @@ -10,6 +10,8 @@ package at.bitfire.davdroid.resource; import android.content.ContentProviderOperation; import android.content.ContentValues; +import android.database.Cursor; +import android.net.Uri; import android.os.RemoteException; import android.provider.ContactsContract; import android.provider.ContactsContract.CommonDataKinds.GroupMembership; @@ -34,10 +36,13 @@ import at.bitfire.vcard4android.CachedGroupMembership; import at.bitfire.vcard4android.Contact; import at.bitfire.vcard4android.ContactsStorageException; import ezvcard.VCardVersion; +import lombok.Cleanup; import static at.bitfire.vcard4android.GroupMethod.GROUP_VCARDS; public class LocalContact extends AndroidContact implements LocalResource { + public static final String COLUMN_HASHCODE = ContactsContract.RawContacts.SYNC3; + protected final Set cachedGroupMemberships = new HashSet<>(), groupMemberships = new HashSet<>(); @@ -61,15 +66,30 @@ public class LocalContact extends AndroidContact implements LocalResource { return TextUtils.isEmpty(getETag()); } + public void resetDirty() throws ContactsStorageException { + ContentValues values = new ContentValues(1); + values.put(ContactsContract.RawContacts.DIRTY, 0); + try { + addressBook.provider.update(rawContactSyncURI(), values, null, null); + } catch(RemoteException e) { + throw new ContactsStorageException("Couldn't clear dirty flag", e); + } + } + public void clearDirty(String eTag) throws ContactsStorageException { try { - ContentValues values = new ContentValues(1); - values.put(ContactsContract.RawContacts.DIRTY, 0); + ContentValues values = new ContentValues(3); values.put(COLUMN_ETAG, eTag); + values.put(ContactsContract.RawContacts.DIRTY, 0); + + int hashCode = dataHashCode(); + values.put(COLUMN_HASHCODE, hashCode); + App.log.finer("Clearing dirty flag with eTag = " + eTag + ", contact hash = " + hashCode); + addressBook.provider.update(rawContactSyncURI(), values, null, null); this.eTag = eTag; - } catch (RemoteException e) { + } catch (FileNotFoundException|RemoteException e) { throw new ContactsStorageException("Couldn't clear dirty flag", e); } } @@ -138,6 +158,53 @@ public class LocalContact extends AndroidContact implements LocalResource { } + @Override + public int update(Contact contact) throws ContactsStorageException { + int result = super.update(contact); + updateHashCode(); + return result; + } + + @Override + public Uri create() throws ContactsStorageException { + Uri uri = super.create(); + updateHashCode(); + return uri; + } + + /** + * Calculates a hash code from the contact's data (VCard) and group memberships. + * @return hash code of contact data (including group memberships) + */ + public int dataHashCode() throws FileNotFoundException, ContactsStorageException { + // groupMemberships is filled by getContact() + return getContact().hashCode() ^ groupMemberships.hashCode(); + } + + protected void updateHashCode() throws ContactsStorageException { + ContentValues values = new ContentValues(1); + try { + int hashCode = dataHashCode(); + App.log.fine("Storing contact hash = " + hashCode); + values.put(COLUMN_HASHCODE, hashCode); + addressBook.provider.update(rawContactSyncURI(), values, null, null); + } catch(FileNotFoundException|RemoteException e) { + throw new ContactsStorageException("Couldn't store contact checksum", e); + } + } + + int getLastHashCode() throws ContactsStorageException { + try { + @Cleanup Cursor c = addressBook.provider.query(rawContactSyncURI(), new String[] { COLUMN_HASHCODE }, null, null, null); + if (c == null || !c.moveToNext() || c.isNull(0)) + return 0; + return c.getInt(0); + } catch(RemoteException e) { + throw new ContactsStorageException("Could't read last hash code", e); + } + } + + public void addToGroup(BatchOperation batch, long groupID) { assertID(); batch.enqueue(new BatchOperation.Operation( diff --git a/app/src/main/java/at/bitfire/davdroid/syncadapter/ContactsSyncManager.java b/app/src/main/java/at/bitfire/davdroid/syncadapter/ContactsSyncManager.java index 751b0931..3a617ca1 100644 --- a/app/src/main/java/at/bitfire/davdroid/syncadapter/ContactsSyncManager.java +++ b/app/src/main/java/at/bitfire/davdroid/syncadapter/ContactsSyncManager.java @@ -10,6 +10,7 @@ package at.bitfire.davdroid.syncadapter; import android.accounts.Account; import android.content.ContentProviderClient; +import android.content.ContentResolver; import android.content.ContentValues; import android.content.Context; import android.content.SyncResult; @@ -81,6 +82,12 @@ public class ContactsSyncManager extends SyncManager { LocalAddressBook localAddressBook = localAddressBook(); localAddressBook.setURL(info.url); + int reallyDirty = localAddressBook.verifyDirty(); + if (extras.containsKey(ContentResolver.SYNC_EXTRAS_UPLOAD) && reallyDirty == 0) { + App.log.info("This sync was called to upload dirty contacts, but no contact data have been changed"); + return false; + } + // set up Contacts Provider Settings ContentValues values = new ContentValues(2); values.put(ContactsContract.Settings.SHOULD_SYNC, 1); diff --git a/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncAdapterService.java b/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncAdapterService.java index caa18079..56a79b14 100644 --- a/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncAdapterService.java +++ b/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncAdapterService.java @@ -74,7 +74,7 @@ public abstract class SyncAdapterService extends Service { @Override public void onPerformSync(Account account, Bundle extras, String authority, ContentProviderClient provider, SyncResult syncResult) { - App.log.info("Sync for " + authority + " has been initiated"); + App.log.log(Level.INFO, "Sync for " + authority + " has been initiated.", extras.keySet().toArray()); // required for dav4android (ServiceLoader) Thread.currentThread().setContextClassLoader(getContext().getClassLoader()); diff --git a/app/src/main/java/at/bitfire/davdroid/ui/AccountSettingsActivity.java b/app/src/main/java/at/bitfire/davdroid/ui/AccountSettingsActivity.java index 499ba60b..42e99229 100644 --- a/app/src/main/java/at/bitfire/davdroid/ui/AccountSettingsActivity.java +++ b/app/src/main/java/at/bitfire/davdroid/ui/AccountSettingsActivity.java @@ -123,7 +123,6 @@ public class AccountSettingsActivity extends AppCompatActivity { @Override public boolean onPreferenceChange(Preference preference, Object newValue) { settings.setSyncInterval(ContactsContract.AUTHORITY, Long.parseLong((String)newValue)); - getLoaderManager().restartLoader(0, getArguments(), AccountSettingsFragment.this); return false; } }); @@ -144,7 +143,6 @@ public class AccountSettingsActivity extends AppCompatActivity { @Override public boolean onPreferenceChange(Preference preference, Object newValue) { settings.setSyncInterval(CalendarContract.AUTHORITY, Long.parseLong((String)newValue)); - getLoaderManager().restartLoader(0, getArguments(), AccountSettingsFragment.this); return false; } }); @@ -165,7 +163,6 @@ public class AccountSettingsActivity extends AppCompatActivity { @Override public boolean onPreferenceChange(Preference preference, Object newValue) { settings.setSyncInterval(TaskProvider.ProviderName.OpenTasks.authority, Long.parseLong((String)newValue)); - getLoaderManager().restartLoader(0, getArguments(), AccountSettingsFragment.this); return false; } });