Add checks for Event validity before committing
* Enforce CalendarProvider2's requirements for valid Events
* Add unit tests for the new code
Bug: 2658149
Change-Id: I42ad7acbcee3b6b831f805c59436017a32651f3a
diff --git a/src/com/android/exchange/adapter/CalendarSyncAdapter.java b/src/com/android/exchange/adapter/CalendarSyncAdapter.java
index b7c2d50..28ac479 100644
--- a/src/com/android/exchange/adapter/CalendarSyncAdapter.java
+++ b/src/com/android/exchange/adapter/CalendarSyncAdapter.java
@@ -133,6 +133,7 @@
Cursor c = mService.mContentResolver.query(Calendars.CONTENT_URI,
new String[] {Calendars._ID}, CALENDAR_SELECTION,
new String[] {mEmailAddress, Email.EXCHANGE_ACCOUNT_MANAGER_TYPE}, null);
+ if (c == null) return;
try {
if (c.moveToFirst()) {
mCalendarId = c.getLong(CALENDAR_SELECTION_ID);
@@ -231,7 +232,7 @@
}
}
- class EasCalendarSyncParser extends AbstractSyncParser {
+ public class EasCalendarSyncParser extends AbstractSyncParser {
String[] mBindArgument = new String[1];
Uri mAccountUri;
@@ -274,6 +275,56 @@
}
}
+ /**
+ * Set DTEND and/or DURATION, as appropriate, for the Event
+ * The follow rules are enforced by CalendarProvider2:
+ * Events MUST have either 1) a DTEND or 2) a DURATION as defined by Rfc2445
+ * Recurring events (i.e. events with RRULE) must have a DURATION
+ * All-day recurring events MUST have a DURATION that is in the form P<n>D
+ * Other events MAY have a DURATION in any valid form (we use P<n>M)
+ * All-day events MUST have hour, minute, and second = 0; in addition, they must have
+ * the TIMEZONE set to UTC
+ * @param cv the ContentValues for the Event
+ * @param startTime the start time for the Event
+ * @param endTime the end time for the Event
+ * @param allDayEvent whether this is an all day event (1) or not (0)
+ */
+ /*package*/ void setTimes(ContentValues cv, long startTime, long endTime, int allDayEvent) {
+ // Sanity check for illegal values; just return (the event will be rejected later by
+ // isValidEventValues()
+ if (startTime < 0 || endTime < 0) return;
+ // If this is an all-day event, set hour, minute, and second to zero
+ if (allDayEvent != 0) {
+ GregorianCalendar cal = new GregorianCalendar(UTC_TIMEZONE);
+ cal.setTimeInMillis(startTime);
+ cal.set(GregorianCalendar.HOUR_OF_DAY, 0);
+ cal.set(GregorianCalendar.MINUTE, 0);
+ cal.set(GregorianCalendar.SECOND, 0);
+ startTime = cal.getTimeInMillis();
+ cal.setTimeInMillis(endTime);
+ cal.set(GregorianCalendar.HOUR_OF_DAY, 0);
+ cal.set(GregorianCalendar.MINUTE, 0);
+ cal.set(GregorianCalendar.SECOND, 0);
+ endTime = cal.getTimeInMillis();
+ cv.put(Events.EVENT_TIMEZONE, UTC_TIMEZONE.getID());
+ }
+ // Always set DTSTART
+ cv.put(Events.DTSTART, startTime);
+ // For recurring events, set DURATION. Use P<n>D format for all day events
+ if (cv.containsKey(Events.RRULE)) {
+ if (allDayEvent != 0) {
+ cv.put(Events.DURATION, "P" + ((endTime - startTime) / DAYS) + "D");
+ }
+ else {
+ cv.put(Events.DURATION, "P" + ((endTime - startTime) / MINUTES) + "M");
+ }
+ // For other events, set DTEND and LAST_DATE
+ } else {
+ cv.put(Events.DTEND, endTime);
+ cv.put(Events.LAST_DATE, endTime);
+ }
+ }
+
public void addEvent(CalendarOperations ops, String serverId, boolean update)
throws IOException {
ContentValues cv = new ContentValues();
@@ -370,7 +421,6 @@
break;
case Tags.CALENDAR_START_TIME:
startTime = Utility.parseDateTimeToMillis(getValue());
- cv.put(Events.DTSTART, startTime);
break;
case Tags.CALENDAR_END_TIME:
endTime = Utility.parseDateTimeToMillis(getValue());
@@ -437,6 +487,9 @@
}
}
+ // Set up DTART and DTEND/DURATION
+ setTimes(cv, startTime, endTime, allDayEvent);
+
// If we haven't added the organizer to attendees, do it now
if (!organizerAdded) {
addOrganizerToAttendees(ops, eventId, organizerName, organizerEmail);
@@ -474,36 +527,6 @@
ops.newExtendedProperty("attendees", sb.toString());
}
- boolean hasRrule = cv.containsKey(Events.RRULE);
- // If there's no recurrence, set DTEND to the end time
- if (!hasRrule) {
- cv.put(Events.DTEND, endTime);
- cv.put(Events.LAST_DATE, endTime);
- }
- // Set the DURATION using rfc2445
- // For all day events, make sure hour, minute, and second are zero for DTSTART/DTEND
- if (allDayEvent != 0) {
- GregorianCalendar cal = new GregorianCalendar(UTC_TIMEZONE);
- cal.setTimeInMillis(startTime);
- cal.set(GregorianCalendar.HOUR_OF_DAY, 0);
- cal.set(GregorianCalendar.MINUTE, 0);
- cal.set(GregorianCalendar.SECOND, 0);
- cv.put(Events.DTSTART, cal.getTimeInMillis());
- // Use DURATION w/ recurrences; otherwise DTEND
- if (!hasRrule) {
- cal.setTimeInMillis(endTime);
- cal.set(GregorianCalendar.HOUR_OF_DAY, 0);
- cal.set(GregorianCalendar.MINUTE, 0);
- cal.set(GregorianCalendar.SECOND, 0);
- cv.put(Events.DTEND, cal.getTimeInMillis());
- } else {
- cv.put(Events.DURATION, "P" + ((endTime - startTime) / DAYS) + "D");
- }
- cv.put(Events.EVENT_TIMEZONE, UTC_TIMEZONE.getID());
- } else {
- cv.put(Events.DURATION, "P" + ((endTime - startTime) / MINUTES) + "M");
- }
-
// Put the real event in the proper place in the ops ArrayList
if (eventOffset >= 0) {
// Store away the DTSTAMP here
@@ -537,11 +560,12 @@
}
}
- private boolean isValidEventValues(ContentValues cv, boolean update) {
- // Do a sanity check on this set of values
+ /*package*/ boolean isValidEventValues(ContentValues cv, boolean update) {
// At the very least, we must get DTSTART and _SYNC_DATA (uid)
- // If it's invalid, log the columns we've got (will help debugging)
- if (!cv.containsKey(Events.DTSTART) || !cv.containsKey(Events._SYNC_DATA)) {
+ // and one of DTEND or DURATION
+ // If the content values are invalid, log the columns (will help debugging)
+ if (!cv.containsKey(Events.DTSTART) || !cv.containsKey(Events._SYNC_DATA) ||
+ (!cv.containsKey(Events.DTEND) && !cv.containsKey(Events.DURATION))) {
userLog(TAG, (update ? "Changed" : "New") + " event invalid; skipping");
StringBuilder sb = new StringBuilder("Columns: ");
for (Entry<String, Object> entry: cv.valueSet()) {
@@ -550,6 +574,16 @@
}
userLog(TAG, sb.toString());
return false;
+ // If we've got an RRULE, we must have a DURATION
+ } else if (cv.containsKey(Events.RRULE)) {
+ String duration = cv.getAsString(Events.DURATION);
+ if (duration == null) return false;
+ if (cv.containsKey(Events.ALL_DAY)) {
+ Integer ade = cv.getAsInteger(Events.ALL_DAY);
+ if (ade != 0 && !duration.endsWith("D")) {
+ return false;
+ }
+ }
}
return true;
}
@@ -618,6 +652,17 @@
cv.put(Events.VISIBILITY, parentCv.getAsString(Events.VISIBILITY));
cv.put(Events.EVENT_TIMEZONE, parentCv.getAsString(Events.EVENT_TIMEZONE));
+ long startTime = -1;
+ long endTime = -1;
+ int allDayEvent = 0;
+
+ if (parentCv.containsKey(Events.DTSTART)) {
+ startTime = parentCv.getAsLong(Events.DTSTART);
+ }
+ if (parentCv.containsKey(Events.DTEND)) {
+ endTime = parentCv.getAsLong(Events.DTEND);
+ }
+
// This column is the key that links the exception to the serverId
cv.put(Events.ORIGINAL_EVENT, parentCv.getAsString(Events._SYNC_ID));
@@ -635,7 +680,8 @@
}
break;
case Tags.CALENDAR_ALL_DAY_EVENT:
- cv.put(Events.ALL_DAY, getValueInt());
+ allDayEvent = getValueInt();
+ cv.put(Events.ALL_DAY, allDayEvent);
break;
case Tags.BASE_BODY:
cv.put(Events.DESCRIPTION, bodyParser());
@@ -644,10 +690,10 @@
cv.put(Events.DESCRIPTION, getValue());
break;
case Tags.CALENDAR_START_TIME:
- cv.put(Events.DTSTART, Utility.parseDateTimeToMillis(getValue()));
+ startTime = Utility.parseDateTimeToMillis(getValue());
break;
case Tags.CALENDAR_END_TIME:
- cv.put(Events.DTEND, Utility.parseDateTimeToMillis(getValue()));
+ endTime = Utility.parseDateTimeToMillis(getValue());
break;
case Tags.CALENDAR_LOCATION:
cv.put(Events.EVENT_LOCATION, getValue());
@@ -685,12 +731,11 @@
cv.put(Events._SYNC_ID, parentCv.getAsString(Events._SYNC_ID) + '_' +
exceptionStartTime);
- if (!cv.containsKey(Events.DTSTART)) {
- cv.put(Events.DTSTART, parentCv.getAsLong(Events.DTSTART));
- }
- if (!cv.containsKey(Events.DTEND)) {
- cv.put(Events.DTEND, parentCv.getAsLong(Events.DTEND));
- }
+ // Set up DTART and DTEND/DURATION
+ setTimes(cv, startTime, endTime, allDayEvent);
+
+ // Don't insert an invalid exception event
+ if (!isValidEventValues(cv, false)) return;
// Add the exception insert
int exceptionStart = ops.mCount;
diff --git a/tests/src/com/android/exchange/adapter/CalendarSyncAdapterTests.java b/tests/src/com/android/exchange/adapter/CalendarSyncAdapterTests.java
index 7f380d5..492e9d6 100644
--- a/tests/src/com/android/exchange/adapter/CalendarSyncAdapterTests.java
+++ b/tests/src/com/android/exchange/adapter/CalendarSyncAdapterTests.java
@@ -16,9 +16,118 @@
package com.android.exchange.adapter;
-public class CalendarSyncAdapterTests extends SyncAdapterTestCase {
+import com.android.exchange.adapter.CalendarSyncAdapter.EasCalendarSyncParser;
+
+import android.content.ContentValues;
+import android.provider.Calendar.Events;
+
+import java.io.IOException;
+import java.util.GregorianCalendar;
+import java.util.TimeZone;
+
+/**
+ * You can run this entire test case with:
+ * runtest -c com.android.exchange.adapter.CalendarSyncAdapterTests email
+ */
+
+public class CalendarSyncAdapterTests extends SyncAdapterTestCase<CalendarSyncAdapter> {
public CalendarSyncAdapterTests() {
super();
}
+
+ public void testSetTimes() throws IOException {
+ CalendarSyncAdapter adapter = getTestSyncAdapter(CalendarSyncAdapter.class);
+ EasCalendarSyncParser p = adapter.new EasCalendarSyncParser(getTestInputStream(), adapter);
+
+ ContentValues cv = new ContentValues();
+
+ // Basic, one-time meeting lasting an hour
+ GregorianCalendar startCalendar = new GregorianCalendar(2010, 5, 10, 8, 30);
+ Long startTime = startCalendar.getTimeInMillis();
+ GregorianCalendar endCalendar = new GregorianCalendar(2010, 5, 10, 9, 30);
+ Long endTime = endCalendar.getTimeInMillis();
+
+ p.setTimes(cv, startTime, endTime, 0);
+ assertNull(cv.getAsInteger(Events.DURATION));
+ assertEquals(startTime, cv.getAsLong(Events.DTSTART));
+ assertEquals(endTime, cv.getAsLong(Events.DTEND));
+ assertEquals(endTime, cv.getAsLong(Events.LAST_DATE));
+ assertNull(cv.getAsString(Events.EVENT_TIMEZONE));
+
+ // Recurring meeting lasting an hour
+ cv.clear();
+ cv.put(Events.RRULE, "FREQ=DAILY");
+ p.setTimes(cv, startTime, endTime, 0);
+ assertEquals("P60M", cv.getAsString(Events.DURATION));
+ assertEquals(startTime, cv.getAsLong(Events.DTSTART));
+ assertNull(cv.getAsLong(Events.DTEND));
+ assertNull(cv.getAsLong(Events.LAST_DATE));
+ assertNull(cv.getAsString(Events.EVENT_TIMEZONE));
+
+ // Recurring all-day event lasting one day
+ cv.clear();
+ startCalendar = new GregorianCalendar(2010, 5, 10, 8, 30);
+ startTime = startCalendar.getTimeInMillis();
+ endCalendar = new GregorianCalendar(2010, 5, 11, 8, 30);
+ endTime = endCalendar.getTimeInMillis();
+ cv.put(Events.RRULE, "FREQ=WEEKLY;BYDAY=MO");
+ p.setTimes(cv, startTime, endTime, 1);
+
+ // The start time should have hour/min/sec zero'd out
+ startCalendar = new GregorianCalendar(TimeZone.getTimeZone("UTC"));
+ startCalendar.set(2010, 5, 10, 0, 0, 0);
+ startCalendar.set(GregorianCalendar.MILLISECOND, 0);
+ startTime = startCalendar.getTimeInMillis();
+ assertEquals(startTime, cv.getAsLong(Events.DTSTART));
+
+ // The duration should be in days
+ assertEquals("P1D", cv.getAsString(Events.DURATION));
+ assertNull(cv.getAsLong(Events.DTEND));
+ assertNull(cv.getAsLong(Events.LAST_DATE));
+ // There must be a timezone
+ assertNotNull(cv.getAsString(Events.EVENT_TIMEZONE));
+ }
+
+ public void testIsValidEventValues() throws IOException {
+ CalendarSyncAdapter adapter = getTestSyncAdapter(CalendarSyncAdapter.class);
+ EasCalendarSyncParser p = adapter.new EasCalendarSyncParser(getTestInputStream(), adapter);
+
+ long validTime = System.currentTimeMillis();
+ String validData = "foo-bar-bletch";
+ String validDuration = "P30M";
+ String validRrule = "FREQ=DAILY";
+
+ ContentValues cv = new ContentValues();
+
+ cv.put(Events.DTSTART, validTime);
+ // Needs _SYNC_DATA and DTEND/DURATION
+ assertFalse(p.isValidEventValues(cv, false));
+ cv.put(Events._SYNC_DATA, validData);
+ // Needs DTEND/DURATION
+ assertFalse(p.isValidEventValues(cv, false));
+ cv.put(Events.DURATION, validDuration);
+ // Valid (DTSTART, _SYNC_DATA, DURATION)
+ assertTrue(p.isValidEventValues(cv, false));
+ cv.remove(Events.DURATION);
+ cv.put(Events.DTEND, validTime);
+ // Valid (DTSTART, _SYNC_DATA, DTEND)
+ assertTrue(p.isValidEventValues(cv, false));
+ cv.remove(Events.DTSTART);
+ // Needs DTSTART
+ assertFalse(p.isValidEventValues(cv, false));
+ cv.put(Events.DTSTART, validTime);
+ cv.put(Events.RRULE, validRrule);
+ // With RRULE, needs DURATION
+ assertFalse(p.isValidEventValues(cv, false));
+ cv.put(Events.DURATION, "P30M");
+ // Valid (RRULE+DURATION)
+ assertTrue(p.isValidEventValues(cv, false));
+ cv.put(Events.ALL_DAY, "1");
+ // Needs DURATION in the form P<n>D
+ assertFalse(p.isValidEventValues(cv, false));
+ // Valid (RRULE+ALLDAY+DURATION(P<n>D)
+ cv.put(Events.DURATION, "P1D");
+ assertTrue(p.isValidEventValues(cv, false));
+ }
}
diff --git a/tests/src/com/android/exchange/adapter/EmailSyncAdapterTests.java b/tests/src/com/android/exchange/adapter/EmailSyncAdapterTests.java
index 6712aa7..7a4ea1b 100644
--- a/tests/src/com/android/exchange/adapter/EmailSyncAdapterTests.java
+++ b/tests/src/com/android/exchange/adapter/EmailSyncAdapterTests.java
@@ -36,7 +36,7 @@
import java.util.GregorianCalendar;
import java.util.TimeZone;
-public class EmailSyncAdapterTests extends SyncAdapterTestCase {
+public class EmailSyncAdapterTests extends SyncAdapterTestCase<EmailSyncAdapter> {
public EmailSyncAdapterTests() {
super();
@@ -73,7 +73,7 @@
}
public void testFormatDateTime() throws IOException {
- EmailSyncAdapter adapter = getTestSyncAdapter();
+ EmailSyncAdapter adapter = getTestSyncAdapter(EmailSyncAdapter.class);
GregorianCalendar calendar = new GregorianCalendar(TimeZone.getTimeZone("GMT"));
// Calendar is odd, months are zero based, so the first 11 below is December...
calendar.set(2008, 11, 11, 18, 19, 20);
diff --git a/tests/src/com/android/exchange/adapter/SyncAdapterTestCase.java b/tests/src/com/android/exchange/adapter/SyncAdapterTestCase.java
index 24cf596..00b6b23 100644
--- a/tests/src/com/android/exchange/adapter/SyncAdapterTestCase.java
+++ b/tests/src/com/android/exchange/adapter/SyncAdapterTestCase.java
@@ -28,9 +28,11 @@
import java.io.ByteArrayInputStream;
import java.io.InputStream;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
-public class SyncAdapterTestCase extends ProviderTestCase2<EmailProvider> {
-
+public class SyncAdapterTestCase<T extends AbstractSyncAdapter>
+ extends ProviderTestCase2<EmailProvider> {
EmailProvider mProvider;
Context mMockContext;
ContentResolver mMockResolver;
@@ -78,13 +80,24 @@
service.mContext = mMockContext;
service.mMailbox = mailbox;
service.mAccount = account;
+ service.mContentResolver = mMockResolver;
return service;
}
- EmailSyncAdapter getTestSyncAdapter() {
+ T getTestSyncAdapter(Class<T> klass) {
EasSyncService service = getTestService();
- EmailSyncAdapter adapter = new EmailSyncAdapter(service.mMailbox, service);
- return adapter;
+ Constructor<T> c;
+ try {
+ c = klass.getDeclaredConstructor(new Class[] {Mailbox.class, EasSyncService.class});
+ return c.newInstance(service.mMailbox, service);
+ } catch (SecurityException e) {
+ } catch (NoSuchMethodException e) {
+ } catch (IllegalArgumentException e) {
+ } catch (InstantiationException e) {
+ } catch (IllegalAccessException e) {
+ } catch (InvocationTargetException e) {
+ }
+ return null;
}
}
