Handle binder transaction failures in CalendarSyncAdapter
* Created a facility for handling the newly implemented
TransactionTooLargeException that the framework can throw if
we send a too-large batch of data to a content provider.
* For now, we're just using this with CalendarSyncAdapter,
which is where we've seen problems in the past, and in
the referenced bug.
* The test account hits the exception, and then appars to
successfully finish syncing the calendar
Bug: 5516422
Change-Id: I3c7ce3ada940464d1ee1f69bd6192640ebbd8fa3
diff --git a/src/com/android/exchange/adapter/AbstractSyncAdapter.java b/src/com/android/exchange/adapter/AbstractSyncAdapter.java
index b4c3c78..52400c4 100644
--- a/src/com/android/exchange/adapter/AbstractSyncAdapter.java
+++ b/src/com/android/exchange/adapter/AbstractSyncAdapter.java
@@ -22,12 +22,21 @@
import com.android.exchange.CommandStatusException;
import com.android.exchange.Eas;
import com.android.exchange.EasSyncService;
+import com.google.common.annotations.VisibleForTesting;
+import android.content.ContentProviderOperation;
+import android.content.ContentProviderResult;
import android.content.ContentResolver;
+import android.content.ContentUris;
import android.content.Context;
+import android.content.OperationApplicationException;
+import android.net.Uri;
+import android.os.RemoteException;
+import android.os.TransactionTooLargeException;
import java.io.IOException;
import java.io.InputStream;
+import java.util.ArrayList;
/**
* Parent class of all sync adapters (EasMailbox, EasCalendar, and EasContacts)
@@ -43,6 +52,8 @@
protected static final String PIM_WINDOW_SIZE = "4";
+ private static final long SEPARATOR_ID = Long.MAX_VALUE;
+
public Mailbox mMailbox;
public EasSyncService mService;
public Context mContext;
@@ -134,5 +145,205 @@
public void setSyncKey(String syncKey, boolean inCommands) throws IOException {
mMailbox.mSyncKey = syncKey;
}
+
+ /**
+ * Operation is our binder-safe ContentProviderOperation (CPO) construct; an Operation can
+ * be created from a CPO, a CPO Builder, or a CPO Builder with a "back reference" column name
+ * and offset (that might be used in Builder.withValueBackReference). The CPO is not actually
+ * built until it is ready to be executed (with applyBatch); this allows us to recalculate
+ * back reference offsets if we are required to re-send a large batch in smaller chunks.
+ *
+ * NOTE: A failed binder transaction is something of an emergency case, and shouldn't happen
+ * with any frequency. When it does, and we are forced to re-send the data to the content
+ * provider in smaller chunks, we DO lose the sync-window atomicity, and thereby add another
+ * small risk to the data. Of course, this is far, far better than dropping the data on the
+ * floor, as was done before the framework implemented TransactionTooLargeException
+ */
+ protected static class Operation {
+ final ContentProviderOperation mOp;
+ final ContentProviderOperation.Builder mBuilder;
+ final String mColumnName;
+ final int mOffset;
+ // Is this Operation a separator? (a good place to break up a large transaction)
+ boolean mSeparator = false;
+
+ // For toString()
+ final String[] TYPES = new String[] {"???", "Ins", "Upd", "Del", "Assert"};
+
+ Operation(ContentProviderOperation.Builder builder, String columnName, int offset) {
+ mOp = null;
+ mBuilder = builder;
+ mColumnName = columnName;
+ mOffset = offset;
+ }
+
+ Operation(ContentProviderOperation.Builder builder) {
+ mOp = null;
+ mBuilder = builder;
+ mColumnName = null;
+ mOffset = 0;
+ }
+
+ Operation(ContentProviderOperation op) {
+ mOp = op;
+ mBuilder = null;
+ mColumnName = null;
+ mOffset = 0;
+ }
+
+ public String toString() {
+ StringBuilder sb = new StringBuilder("Op: ");
+ ContentProviderOperation op = operationToContentProviderOperation(this, 0);
+ int type = 0;
+ //DO NOT SHIP WITH THE FOLLOWING LINE (the API is hidden!)
+ //type = op.getType();
+ sb.append(TYPES[type]);
+ Uri uri = op.getUri();
+ sb.append(' ');
+ sb.append(uri.getPath());
+ if (mColumnName != null) {
+ sb.append(" Back value of " + mColumnName + ": " + mOffset);
+ }
+ return sb.toString();
+ }
+ }
+
+ /**
+ * We apply the batch of CPO's here. We synchronize on the service to avoid thread-nasties,
+ * and we just return quickly if the service has already been stopped.
+ */
+ private ContentProviderResult[] execute(String authority,
+ ArrayList<ContentProviderOperation> ops)
+ throws RemoteException, OperationApplicationException {
+ synchronized (mService.getSynchronizer()) {
+ if (!mService.isStopped()) {
+ if (!ops.isEmpty()) {
+ ContentProviderResult[] result = mContentResolver.applyBatch(authority, ops);
+ mService.userLog("Results: " + result.length);
+ return result;
+ }
+ }
+ }
+ return new ContentProviderResult[0];
+ }
+
+ /**
+ * Convert an Operation to a CPO; if the Operation has a back reference, apply it with the
+ * passed-in offset
+ */
+ @VisibleForTesting
+ static ContentProviderOperation operationToContentProviderOperation(Operation op, int offset) {
+ if (op.mOp != null) {
+ return op.mOp;
+ } else if (op.mBuilder == null) {
+ throw new IllegalArgumentException("Operation must have CPO.Builder");
+ }
+ ContentProviderOperation.Builder builder = op.mBuilder;
+ if (op.mColumnName != null) {
+ builder.withValueBackReference(op.mColumnName, op.mOffset - offset);
+ }
+ return builder.build();
+ }
+
+ /**
+ * Create a list of CPOs from a list of Operations, and then apply them in a batch
+ */
+ private ContentProviderResult[] applyBatch(String authority, ArrayList<Operation> ops,
+ int offset) throws RemoteException, OperationApplicationException {
+ // Handle the empty case
+ if (ops.isEmpty()) {
+ return new ContentProviderResult[0];
+ }
+ ArrayList<ContentProviderOperation> cpos = new ArrayList<ContentProviderOperation>();
+ for (Operation op: ops) {
+ cpos.add(operationToContentProviderOperation(op, offset));
+ }
+ return execute(authority, cpos);
+ }
+
+ /**
+ * Apply the list of CPO's in the provider and copy the "mini" result into our full result array
+ */
+ private void applyAndCopyResults(String authority, ArrayList<Operation> mini,
+ ContentProviderResult[] result, int offset) throws RemoteException {
+ // Empty lists are ok; we just ignore them
+ if (mini.isEmpty()) return;
+ try {
+ ContentProviderResult[] miniResult = applyBatch(authority, mini, offset);
+ // Copy the results from this mini-batch into our results array
+ System.arraycopy(miniResult, 0, result, offset, miniResult.length);
+ } catch (OperationApplicationException e) {
+ // Not possible since we're building the ops ourselves
+ }
+ }
+
+ /**
+ * Called by a sync adapter to execute a list of Operations in the ContentProvider handling
+ * the passed-in authority. If the attempt to apply the batch fails due to a too-large
+ * binder transaction, we split the Operations as directed by separators. If any of the
+ * "mini" batches fails due to a too-large transaction, we're screwed, but this would be
+ * vanishingly rare. Other, possibly transient, errors are handled by throwing a
+ * RemoteException, which the caller will likely re-throw as an IOException so that the sync
+ * can be attempted again.
+ *
+ * Callers MAY leave a dangling separator at the end of the list; note that the separators
+ * themselves are only markers and are not sent to the provider.
+ */
+ protected ContentProviderResult[] safeExecute(String authority, ArrayList<Operation> ops)
+ throws RemoteException {
+ mService.userLog("Try to execute ", ops.size(), " CPO's for " + authority);
+ ContentProviderResult[] result = null;
+ try {
+ // Try to execute the whole thing
+ return applyBatch(authority, ops, 0);
+ } catch (TransactionTooLargeException e) {
+ // Nope; split into smaller chunks, demarcated by the separator operation
+ mService.userLog("Transaction too large; spliting!");
+ ArrayList<Operation> mini = new ArrayList<Operation>();
+ // Build a result array with the total size we're sending
+ result = new ContentProviderResult[ops.size()];
+ int count = 0;
+ int offset = 0;
+ for (Operation op: ops) {
+ if (op.mSeparator) {
+ try {
+ mService.userLog("Try mini-batch of ", mini.size(), " CPO's");
+ applyAndCopyResults(authority, mini, result, offset);
+ mini.clear();
+ // Save away the offset here; this will need to be subtracted out of the
+ // value originally set by the adapter
+ offset = count + 1; // Remember to add 1 for the separator!
+ } catch (TransactionTooLargeException e1) {
+ throw new RuntimeException("Can't send transaction; sync stopped.");
+ } catch (RemoteException e1) {
+ throw e1;
+ }
+ } else {
+ mini.add(op);
+ }
+ count++;
+ }
+ // Check out what's left; if it's more than just a separator, apply the batch
+ int miniSize = mini.size();
+ if ((miniSize > 0) && !(miniSize == 1 && mini.get(0).mSeparator)) {
+ applyAndCopyResults(authority, mini, result, offset);
+ }
+ } catch (RemoteException e) {
+ throw e;
+ } catch (OperationApplicationException e) {
+ // Not possible since we're building the ops ourselves
+ }
+ return result;
+ }
+
+ /**
+ * Called by a sync adapter to indicate a relatively safe place to split a batch of CPO's
+ */
+ protected void addSeparatorOperation(ArrayList<Operation> ops, Uri uri) {
+ Operation op = new Operation(
+ ContentProviderOperation.newDelete(ContentUris.withAppendedId(uri, SEPARATOR_ID)));
+ op.mSeparator = true;
+ ops.add(op);
+ }
}
diff --git a/src/com/android/exchange/adapter/CalendarSyncAdapter.java b/src/com/android/exchange/adapter/CalendarSyncAdapter.java
index b467eb7..c899640 100644
--- a/src/com/android/exchange/adapter/CalendarSyncAdapter.java
+++ b/src/com/android/exchange/adapter/CalendarSyncAdapter.java
@@ -26,7 +26,6 @@
import android.content.Entity;
import android.content.Entity.NamedContentValues;
import android.content.EntityIterator;
-import android.content.OperationApplicationException;
import android.database.Cursor;
import android.database.DatabaseUtils;
import android.net.Uri;
@@ -131,8 +130,8 @@
// Used to indicate that upsyncs aren't allowed (we catch this in sendLocalChanges)
private static final String EXTENDED_PROPERTY_UPSYNC_PROHIBITED = "upsyncProhibited";
- private static final ContentProviderOperation PLACEHOLDER_OPERATION =
- ContentProviderOperation.newInsert(Uri.EMPTY).build();
+ private static final Operation PLACEHOLDER_OPERATION =
+ new Operation(ContentProviderOperation.newInsert(Uri.EMPTY));
private static final Object sSyncKeyLock = new Object();
@@ -164,9 +163,25 @@
private ArrayList<Long> mSendCancelIdList = new ArrayList<Long>();
private ArrayList<Message> mOutgoingMailList = new ArrayList<Message>();
+ private final Uri mAsSyncAdapterAttendees;
+ private final Uri mAsSyncAdapterEvents;
+ private final Uri mAsSyncAdapterReminders;
+ private final Uri mAsSyncAdapterExtendedProperties;
+
public CalendarSyncAdapter(EasSyncService service) {
super(service);
mEmailAddress = mAccount.mEmailAddress;
+
+ String amType = Eas.EXCHANGE_ACCOUNT_MANAGER_TYPE;
+ mAsSyncAdapterAttendees =
+ asSyncAdapter(Attendees.CONTENT_URI, mEmailAddress, amType);
+ mAsSyncAdapterEvents =
+ asSyncAdapter(Events.CONTENT_URI, mEmailAddress, amType);
+ mAsSyncAdapterReminders =
+ asSyncAdapter(Reminders.CONTENT_URI, mEmailAddress, amType);
+ mAsSyncAdapterExtendedProperties =
+ asSyncAdapter(ExtendedProperties.CONTENT_URI, mEmailAddress, amType);
+
Cursor c = mService.mContentResolver.query(Calendars.CONTENT_URI,
new String[] {Calendars._ID}, CALENDAR_SELECTION,
new String[] {mEmailAddress, Eas.EXCHANGE_ACCOUNT_MANAGER_TYPE}, null);
@@ -182,7 +197,7 @@
} finally {
c.close();
}
- }
+ }
@Override
public String getCollectionName() {
@@ -445,12 +460,9 @@
// This is an attendees-only update; just
// delete/re-add attendees
mBindArgument[0] = Long.toString(id);
- ops.add(ContentProviderOperation
- .newDelete(
- asSyncAdapter(Attendees.CONTENT_URI, mEmailAddress,
- Eas.EXCHANGE_ACCOUNT_MANAGER_TYPE))
- .withSelection(ATTENDEES_EXCEPT_ORGANIZER, mBindArgument)
- .build());
+ ops.add(new Operation(ContentProviderOperation
+ .newDelete(mAsSyncAdapterAttendees)
+ .withSelection(ATTENDEES_EXCEPT_ORGANIZER, mBindArgument)));
eventId = id;
} else {
// Otherwise, delete the original event and recreate it
@@ -692,11 +704,8 @@
if (isValidEventValues(cv)) {
ops.set(eventOffset,
- ContentProviderOperation
- .newInsert(
- asSyncAdapter(Events.CONTENT_URI, mEmailAddress,
- Eas.EXCHANGE_ACCOUNT_MANAGER_TYPE))
- .withValues(cv).build());
+ new Operation(ContentProviderOperation
+ .newInsert(mAsSyncAdapterEvents).withValues(cv)));
} else {
// If we can't add this event (it's invalid), remove all of the inserts
// we've built for it
@@ -718,6 +727,8 @@
}
}
}
+ // Mark the end of the event
+ addSeparatorOperation(ops, Events.CONTENT_URI);
}
private void logEventColumns(ContentValues cv, String reason) {
@@ -1187,11 +1198,11 @@
public void commit() throws IOException {
userLog("Calendar SyncKey saved as: ", mMailbox.mSyncKey);
// Save the syncKey here, using the Helper provider by Calendar provider
- mOps.add(SyncStateContract.Helpers.newSetOperation(
+ mOps.add(new Operation(SyncStateContract.Helpers.newSetOperation(
asSyncAdapter(SyncState.CONTENT_URI, mEmailAddress,
Eas.EXCHANGE_ACCOUNT_MANAGER_TYPE),
mAccountManagerAccount,
- mMailbox.mSyncKey.getBytes()));
+ mMailbox.mSyncKey.getBytes())));
// We need to send cancellations now, because the Event won't exist after the commit
for (long eventId: mSendCancelIdList) {
@@ -1209,8 +1220,12 @@
}
}
- // Execute these all at once...
- mOps.execute();
+ // Execute our CPO's safely
+ try {
+ mOps.mResults = safeExecute(CalendarContract.AUTHORITY, mOps);
+ } catch (RemoteException e) {
+ throw new IOException("Remote exception caught; will retry");
+ }
if (mOps.mResults != null) {
// Clear dirty and mark flags for updates sent to server
@@ -1280,12 +1295,9 @@
cv.put(Events.SYNC_DATA2, clientId);
long id = c.getLong(0);
// Write the serverId into the Event
- mOps.add(ContentProviderOperation
- .newUpdate(
- asSyncAdapter(
- ContentUris.withAppendedId(Events.CONTENT_URI, id),
- mEmailAddress, Eas.EXCHANGE_ACCOUNT_MANAGER_TYPE))
- .withValues(cv).build());
+ mOps.add(new Operation(ContentProviderOperation
+ .newUpdate(ContentUris.withAppendedId(mAsSyncAdapterEvents, id))
+ .withValues(cv)));
userLog("New event " + clientId + " was given serverId: " + serverId);
}
} finally {
@@ -1328,20 +1340,20 @@
}
}
- protected class CalendarOperations extends ArrayList<ContentProviderOperation> {
+ protected class CalendarOperations extends ArrayList<Operation> {
private static final long serialVersionUID = 1L;
public int mCount = 0;
private ContentProviderResult[] mResults = null;
private int mEventStart = 0;
@Override
- public boolean add(ContentProviderOperation op) {
+ public boolean add(Operation op) {
super.add(op);
mCount++;
return true;
}
- public int newEvent(ContentProviderOperation op) {
+ public int newEvent(Operation op) {
mEventStart = mCount;
add(op);
return mEventStart;
@@ -1358,31 +1370,29 @@
}
public void newAttendee(ContentValues cv, int eventStart) {
- add(ContentProviderOperation
- .newInsert(asSyncAdapter(Attendees.CONTENT_URI, mEmailAddress,
- Eas.EXCHANGE_ACCOUNT_MANAGER_TYPE)).withValues(cv)
- .withValueBackReference(Attendees.EVENT_ID, eventStart).build());
+ add(new Operation(ContentProviderOperation.newInsert(mAsSyncAdapterAttendees)
+ .withValues(cv),
+ Attendees.EVENT_ID,
+ eventStart));
}
public void updatedAttendee(ContentValues cv, long id) {
cv.put(Attendees.EVENT_ID, id);
- add(ContentProviderOperation.newInsert(asSyncAdapter(Attendees.CONTENT_URI,
- mEmailAddress, Eas.EXCHANGE_ACCOUNT_MANAGER_TYPE)).withValues(cv).build());
+ add(new Operation(ContentProviderOperation.newInsert(mAsSyncAdapterAttendees)
+ .withValues(cv)));
}
public void newException(ContentValues cv) {
- add(ContentProviderOperation.newInsert(
- asSyncAdapter(Events.CONTENT_URI, mEmailAddress,
- Eas.EXCHANGE_ACCOUNT_MANAGER_TYPE)).withValues(cv).build());
+ add(new Operation(ContentProviderOperation.newInsert(mAsSyncAdapterEvents)
+ .withValues(cv)));
}
public void newExtendedProperty(String name, String value) {
- add(ContentProviderOperation
- .newInsert(asSyncAdapter(ExtendedProperties.CONTENT_URI, mEmailAddress,
- Eas.EXCHANGE_ACCOUNT_MANAGER_TYPE))
+ add(new Operation(ContentProviderOperation.newInsert(mAsSyncAdapterExtendedProperties)
.withValue(ExtendedProperties.NAME, name)
- .withValue(ExtendedProperties.VALUE, value)
- .withValueBackReference(ExtendedProperties.EVENT_ID, mEventStart).build());
+ .withValue(ExtendedProperties.VALUE, value),
+ ExtendedProperties.EVENT_ID,
+ mEventStart));
}
public void updatedExtendedProperty(String name, String value, long id) {
@@ -1404,25 +1414,22 @@
// Either do an update or an insert, depending on whether one
// already exists
if (extendedPropertyId >= 0) {
- add(ContentProviderOperation
+ add(new Operation(ContentProviderOperation
.newUpdate(
- ContentUris.withAppendedId(
- asSyncAdapter(ExtendedProperties.CONTENT_URI,
- mEmailAddress, Eas.EXCHANGE_ACCOUNT_MANAGER_TYPE),
+ ContentUris.withAppendedId(mAsSyncAdapterExtendedProperties,
extendedPropertyId))
- .withValue(ExtendedProperties.VALUE, value).build());
+ .withValue(ExtendedProperties.VALUE, value)));
} else {
newExtendedProperty(name, value);
}
}
public void newReminder(int mins, int eventStart) {
- add(ContentProviderOperation
- .newInsert(asSyncAdapter(Reminders.CONTENT_URI, mEmailAddress,
- Eas.EXCHANGE_ACCOUNT_MANAGER_TYPE))
+ add(new Operation(ContentProviderOperation.newInsert(mAsSyncAdapterReminders)
.withValue(Reminders.MINUTES, mins)
- .withValue(Reminders.METHOD, Reminders.METHOD_ALERT)
- .withValueBackReference(ExtendedProperties.EVENT_ID, eventStart).build());
+ .withValue(Reminders.METHOD, Reminders.METHOD_ALERT),
+ ExtendedProperties.EVENT_ID,
+ eventStart));
}
public void newReminder(int mins) {
@@ -1430,35 +1437,12 @@
}
public void delete(long id, String syncId) {
- add(ContentProviderOperation.newDelete(
- asSyncAdapter(ContentUris.withAppendedId(Events.CONTENT_URI, id),
- mEmailAddress, Eas.EXCHANGE_ACCOUNT_MANAGER_TYPE)).build());
- // Delete the exceptions for this Event (CalendarProvider doesn't do
- // this)
- add(ContentProviderOperation
- .newDelete(asSyncAdapter(Events.CONTENT_URI, mEmailAddress,
- Eas.EXCHANGE_ACCOUNT_MANAGER_TYPE))
- .withSelection(Events.ORIGINAL_SYNC_ID + "=?", new String[] {syncId}).build());
- }
-
- public void execute() {
- synchronized (mService.getSynchronizer()) {
- if (!mService.isStopped()) {
- try {
- if (!isEmpty()) {
- mService.userLog("Executing ", size(), " CPO's");
- mResults = mContext.getContentResolver().applyBatch(
- CalendarContract.AUTHORITY, this);
- }
- } catch (RemoteException e) {
- // There is nothing sensible to be done here
- Log.e(TAG, "problem inserting event during server update", e);
- } catch (OperationApplicationException e) {
- // There is nothing sensible to be done here
- Log.e(TAG, "problem inserting event during server update", e);
- }
- }
- }
+ add(new Operation(ContentProviderOperation.newDelete(
+ ContentUris.withAppendedId(mAsSyncAdapterEvents, id))));
+ // Delete the exceptions for this Event (CalendarProvider doesn't do this)
+ add(new Operation(ContentProviderOperation
+ .newDelete(mAsSyncAdapterEvents)
+ .withSelection(Events.ORIGINAL_SYNC_ID + "=?", new String[] {syncId})));
}
}
diff --git a/tests/src/com/android/exchange/adapter/CalendarSyncAdapterTests.java b/tests/src/com/android/exchange/adapter/CalendarSyncAdapterTests.java
index 4bb93f6..2e2f553 100644
--- a/tests/src/com/android/exchange/adapter/CalendarSyncAdapterTests.java
+++ b/tests/src/com/android/exchange/adapter/CalendarSyncAdapterTests.java
@@ -16,6 +16,7 @@
package com.android.exchange.adapter;
+import com.android.exchange.adapter.AbstractSyncAdapter.Operation;
import com.android.exchange.adapter.CalendarSyncAdapter.CalendarOperations;
import com.android.exchange.adapter.CalendarSyncAdapter.EasCalendarSyncParser;
import com.android.exchange.provider.MockProvider;
@@ -38,6 +39,7 @@
import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.IOException;
+import java.util.ArrayList;
import java.util.GregorianCalendar;
import java.util.List;
import java.util.TimeZone;
@@ -266,10 +268,12 @@
private int countInsertOperationsForTable(CalendarOperations ops, String tableName) {
int cnt = 0;
- for (ContentProviderOperation op: ops) {
- List<String> segments = op.getUri().getPathSegments();
+ for (Operation op: ops) {
+ ContentProviderOperation cpo =
+ AbstractSyncAdapter.operationToContentProviderOperation(op, 0);
+ List<String> segments = cpo.getUri().getPathSegments();
if (segments.get(0).equalsIgnoreCase(tableName) &&
- op.getType() == ContentProviderOperation.TYPE_INSERT) {
+ cpo.getType() == ContentProviderOperation.TYPE_INSERT) {
cnt++;
}
}
@@ -420,7 +424,11 @@
EasCalendarSyncParser p = event.getParser();
p.addEvent(p.mOps, "1:1", update);
// Send the CPO's to the mock provider
- mMockResolver.applyBatch(MockProvider.AUTHORITY, p.mOps);
+ ArrayList<ContentProviderOperation> cpos = new ArrayList<ContentProviderOperation>();
+ for (Operation op: p.mOps) {
+ cpos.add(AbstractSyncAdapter.operationToContentProviderOperation(op, 0));
+ }
+ mMockResolver.applyBatch(MockProvider.AUTHORITY, cpos);
return mMockResolver.query(MockProvider.uri(Attendees.CONTENT_URI), ATTENDEE_PROJECTION,
null, null, null);
}
