From 4e0cd7f554d5f5b322b565ff95209a70df16819e Mon Sep 17 00:00:00 2001 From: Tom Hacohen Date: Thu, 6 Apr 2017 23:02:01 +0100 Subject: [PATCH] SyncManager: Use the last journal id as the ctag, instead of storing it. We were storing the ctag separately although the data was already present in the journal. The last entry's ID is always the CTAG. This could cause issues if sync is aborted exactly at the right time. I managed to trigger this issue on rare cases. --- .../syncadapter/model/JournalModel.java | 9 ++++ .../resource/LocalAddressBook.java | 22 +-------- .../syncadapter/resource/LocalCalendar.java | 29 +----------- .../syncadapter/resource/LocalCollection.java | 3 -- .../syncadapter/resource/LocalTaskList.java | 25 ----------- .../syncadapter/syncadapter/SyncManager.java | 45 +++++++------------ 6 files changed, 30 insertions(+), 103 deletions(-) diff --git a/app/src/main/java/com/etesync/syncadapter/model/JournalModel.java b/app/src/main/java/com/etesync/syncadapter/model/JournalModel.java index e0888777..fa19b868 100644 --- a/app/src/main/java/com/etesync/syncadapter/model/JournalModel.java +++ b/app/src/main/java/com/etesync/syncadapter/model/JournalModel.java @@ -82,6 +82,15 @@ public class JournalModel { } return journalEntity; } + + public String getLastUid(EntityDataStore data) { + EntryEntity last = data.select(EntryEntity.class).where(EntryEntity.JOURNAL.eq(this)).orderBy(EntryEntity.ID.desc()).limit(1).get().firstOrNull(); + if (last != null) { + return last.getUid(); + } + + return null; + } } @Entity diff --git a/app/src/main/java/com/etesync/syncadapter/resource/LocalAddressBook.java b/app/src/main/java/com/etesync/syncadapter/resource/LocalAddressBook.java index 7345ef46..d6b8e8ca 100644 --- a/app/src/main/java/com/etesync/syncadapter/resource/LocalAddressBook.java +++ b/app/src/main/java/com/etesync/syncadapter/resource/LocalAddressBook.java @@ -304,24 +304,6 @@ public class LocalAddressBook extends AndroidAddressBook implements LocalCollect } } - @Override - public String getCTag() throws ContactsStorageException { - synchronized (syncState) { - readSyncState(); - return syncState.getString(SYNC_STATE_CTAG); - } - } - - @Override - public void setCTag(String cTag) throws ContactsStorageException { - synchronized (syncState) { - readSyncState(); - syncState.putString(SYNC_STATE_CTAG, cTag); - writeSyncState(); - } - } - - // HELPERS public static void onRenameAccount(@NonNull ContentResolver resolver, @NonNull String oldName, @NonNull String newName) throws RemoteException { @@ -334,9 +316,9 @@ public class LocalAddressBook extends AndroidAddressBook implements LocalCollect } /** Fix all of the etags of all of the non-dirty contacts to be non-null. - * Currently set to the ctag. */ + * Currently set to all ones. */ public void fixEtags() throws ContactsStorageException { - String newEtag = getCTag(); + String newEtag = "1111111111111111111111111111111111111111111111111111111111111111"; String where = ContactsContract.RawContacts.DIRTY + "=0 AND " + AndroidContact.COLUMN_ETAG + " IS NULL"; ContentValues values = new ContentValues(1); diff --git a/app/src/main/java/com/etesync/syncadapter/resource/LocalCalendar.java b/app/src/main/java/com/etesync/syncadapter/resource/LocalCalendar.java index 8f3cba83..187a2fea 100644 --- a/app/src/main/java/com/etesync/syncadapter/resource/LocalCalendar.java +++ b/app/src/main/java/com/etesync/syncadapter/resource/LocalCalendar.java @@ -160,31 +160,6 @@ public class LocalCalendar extends AndroidCalendar implements LocalCollection { return dirty.toArray(new LocalResource[dirty.size()]); } - - @Override - @SuppressWarnings("Recycle") - public String getCTag() throws CalendarStorageException { - try { - @Cleanup Cursor cursor = provider.query(calendarSyncURI(), new String[] { COLUMN_CTAG }, null, null, null); - if (cursor != null && cursor.moveToNext()) - return cursor.getString(0); - } catch (RemoteException e) { - throw new CalendarStorageException("Couldn't read local (last known) CTag", e); - } - return null; - } - - @Override - public void setCTag(String cTag) throws CalendarStorageException, ContactsStorageException { - try { - ContentValues values = new ContentValues(1); - values.put(COLUMN_CTAG, cTag); - provider.update(calendarSyncURI(), values, null, null); - } catch (RemoteException e) { - throw new CalendarStorageException("Couldn't write local (last known) CTag", e); - } - } - @SuppressWarnings("Recycle") public void processDirtyExceptions() throws CalendarStorageException { // process deleted exceptions @@ -286,9 +261,9 @@ public class LocalCalendar extends AndroidCalendar implements LocalCollection { } /** Fix all of the etags of all of the non-dirty events to be non-null. - * Currently set to the ctag. */ + * Currently set to all ones.. */ public void fixEtags() throws CalendarStorageException { - String newEtag = getCTag(); + String newEtag = "1111111111111111111111111111111111111111111111111111111111111111"; String where = Events.CALENDAR_ID + "=? AND " + Events.DIRTY + "=0 AND " + LocalEvent.COLUMN_ETAG + " IS NULL"; String whereArgs[] = {String.valueOf(id)}; diff --git a/app/src/main/java/com/etesync/syncadapter/resource/LocalCollection.java b/app/src/main/java/com/etesync/syncadapter/resource/LocalCollection.java index a17a5d77..d5c7c88f 100644 --- a/app/src/main/java/com/etesync/syncadapter/resource/LocalCollection.java +++ b/app/src/main/java/com/etesync/syncadapter/resource/LocalCollection.java @@ -22,8 +22,5 @@ public interface LocalCollection { LocalResource getByUid(String uid) throws CalendarStorageException, ContactsStorageException; - String getCTag() throws CalendarStorageException, ContactsStorageException; - void setCTag(String cTag) throws CalendarStorageException, ContactsStorageException; - long count() throws CalendarStorageException, ContactsStorageException; } diff --git a/app/src/main/java/com/etesync/syncadapter/resource/LocalTaskList.java b/app/src/main/java/com/etesync/syncadapter/resource/LocalTaskList.java index 23543194..3cf69094 100644 --- a/app/src/main/java/com/etesync/syncadapter/resource/LocalTaskList.java +++ b/app/src/main/java/com/etesync/syncadapter/resource/LocalTaskList.java @@ -126,31 +126,6 @@ public class LocalTaskList extends AndroidTaskList implements LocalCollection { } } - @Override - @SuppressWarnings("Recycle") - public String getCTag() throws CalendarStorageException { - try { - @Cleanup Cursor cursor = provider.client.query(taskListSyncUri(), new String[] { COLUMN_CTAG }, null, null, null); - if (cursor != null && cursor.moveToNext()) - return cursor.getString(0); - } catch (RemoteException e) { - throw new CalendarStorageException("Couldn't read local (last known) CTag", e); - } - return null; - } - - @Override - public void setCTag(String cTag) throws CalendarStorageException { - try { - ContentValues values = new ContentValues(1); - values.put(COLUMN_CTAG, cTag); - provider.client.update(taskListSyncUri(), values, null, null); - } catch (RemoteException e) { - throw new CalendarStorageException("Couldn't write local (last known) CTag", e); - } - } - - // helpers public static boolean tasksProviderAvailable(@NonNull Context context) { diff --git a/app/src/main/java/com/etesync/syncadapter/syncadapter/SyncManager.java b/app/src/main/java/com/etesync/syncadapter/syncadapter/SyncManager.java index 18d26190..cf07d4a0 100644 --- a/app/src/main/java/com/etesync/syncadapter/syncadapter/SyncManager.java +++ b/app/src/main/java/com/etesync/syncadapter/syncadapter/SyncManager.java @@ -181,6 +181,8 @@ abstract public class SyncManager { syncPhase = R.string.sync_phase_post_processing; App.log.info("Sync phase: " + context.getString(syncPhase)); postProcess(); + + App.log.info("Finished sync with CTag=" + remoteCTag); } catch (IOException e) { App.log.log(Level.WARNING, "I/O exception during sync, trying again later", e); syncResult.stats.numIoExceptions++; @@ -295,24 +297,20 @@ abstract public class SyncManager { String strTotal = String.valueOf(remoteEntries.size()); int i = 0; - try { - for (JournalEntryManager.Entry entry : remoteEntries) { - if (Thread.interrupted()) { - throw new InterruptedException(); - } - i++; - App.log.info("Processing (" + String.valueOf(i) + "/" + strTotal + ") " + entry.toString()); - - SyncEntry cEntry = SyncEntry.fromJournalEntry(crypto, entry); - App.log.info("Processing resource for journal entry"); - processSyncEntry(cEntry); - - persistSyncEntry(entry.getUid(), cEntry); - - remoteCTag = entry.getUid(); + for (JournalEntryManager.Entry entry : remoteEntries) { + if (Thread.interrupted()) { + throw new InterruptedException(); } - } finally { - saveSyncTag(); + i++; + App.log.info("Processing (" + String.valueOf(i) + "/" + strTotal + ") " + entry.toString()); + + SyncEntry cEntry = SyncEntry.fromJournalEntry(crypto, entry); + App.log.info("Processing resource for journal entry"); + processSyncEntry(cEntry); + + persistSyncEntry(entry.getUid(), cEntry); + + remoteCTag = entry.getUid(); } } @@ -352,8 +350,6 @@ abstract public class SyncManager { localDirty = null; localDeleted = null; - - saveSyncTag(); } } @@ -390,12 +386,12 @@ abstract public class SyncManager { /** */ protected void prepareLocal() throws CalendarStorageException, ContactsStorageException, FileNotFoundException { + remoteCTag = getJournalEntity().getLastUid(data); + localDeleted = processLocallyDeleted(); localDirty = localCollection.getDirty(); // This is done after fetching the local dirty so all the ones we are using will be prepared prepareDirty(); - - remoteCTag = localCollection.getCTag(); } @@ -438,11 +434,4 @@ abstract public class SyncManager { */ protected void postProcess() throws CalendarStorageException, ContactsStorageException { } - - private void saveSyncTag() throws CalendarStorageException, ContactsStorageException { - App.log.info("Saving CTag=" + remoteCTag); - localCollection.setCTag(remoteCTag); - } - - }