Fix issues with handling FREQ=YEARLY in RRULEs

* Turns out that we weren't handling one of the cases for YEARLY
  RRULE; that in which the date is specified as a day of week plus
  week of month
* Also, we weren't always sending the INTERVAL properly in a few
  cases
* Fix these issues and add a unit test that confirms the fixes
* Also removed an unused argument in recurrenceParser in
  CalendarSyncAdapter

Bug: 2718948

Change-Id: If9146d484218e7d6bd9f5c2305d0e6a216aeed49

src/com/android/exchange/ExchangeService.java

@@ -107,7 +107,7 @@ import java.util.List;
  */
 public class ExchangeService extends Service implements Runnable {
 
-    private static final String TAG = "EAS ExchangeService";
+    private static final String TAG = "ExchangeService";
 
     // The ExchangeService's mailbox "id"
     public static final int EXTRA_MAILBOX_ID = -1;

src/com/android/exchange/adapter/CalendarSyncAdapter.java

@@ -499,7 +499,7 @@ public class CalendarSyncAdapter extends AbstractSyncAdapter {
                         cv.put(Events.EVENT_LOCATION, getValue());
                         break;
                     case Tags.CALENDAR_RECURRENCE:
-                        String rrule = recurrenceParser(ops);
+                        String rrule = recurrenceParser();
                         if (rrule != null) {
                             cv.put(Events.RRULE, rrule);
                         }
@@ -723,7 +723,7 @@ public class CalendarSyncAdapter extends AbstractSyncAdapter {
             return true;
         }
 
-        private String recurrenceParser(CalendarOperations ops) throws IOException {
+        public String recurrenceParser() throws IOException {
             // Turn this information into an RRULE
             int type = -1;
             int occurrences = -1;
@@ -830,7 +830,7 @@ public class CalendarSyncAdapter extends AbstractSyncAdapter {
                         cv.put(Events.EVENT_LOCATION, getValue());
                         break;
                     case Tags.CALENDAR_RECURRENCE:
-                        String rrule = recurrenceParser(ops);
+                        String rrule = recurrenceParser();
                         if (rrule != null) {
                             cv.put(Events.RRULE, rrule);
                         }

src/com/android/exchange/utility/CalendarUtilities.java

@@ -1016,15 +1016,40 @@ public class CalendarUtilities {
     }
 
     /**
-     * Convenience method to add "until" to an EAS calendar stream
+     * Convenience method to add "count", "interval", and "until" to an EAS calendar stream
+     * According to EAS docs, OCCURRENCES must always come before INTERVAL
      */
-    static void addUntil(String rrule, Serializer s) throws IOException {
+    static private void addCountIntervalAndUntil(String rrule, Serializer s) throws IOException {
+        String count = tokenFromRrule(rrule, "COUNT=");
+        if (count != null) {
+            s.data(Tags.CALENDAR_RECURRENCE_OCCURRENCES, count);
+        }
+        String interval = tokenFromRrule(rrule, "INTERVAL=");
+        if (interval != null) {
+            s.data(Tags.CALENDAR_RECURRENCE_INTERVAL, interval);
+        }
         String until = tokenFromRrule(rrule, "UNTIL=");
         if (until != null) {
             s.data(Tags.CALENDAR_RECURRENCE_UNTIL, recurrenceUntilToEasUntil(until));
         }
     }
 
+    static private void addByDay(String byDay, Serializer s) throws IOException {
+        // This can be 1WE (1st Wednesday) or -1FR (last Friday)
+        int weekOfMonth = byDay.charAt(0);
+        String bareByDay;
+        if (weekOfMonth == '-') {
+            // -1 is the only legal case (last week) Use "5" for EAS
+            weekOfMonth = 5;
+            bareByDay = byDay.substring(2);
+        } else {
+            weekOfMonth = weekOfMonth - '0';
+            bareByDay = byDay.substring(1);
+        }
+        s.data(Tags.CALENDAR_RECURRENCE_WEEKOFMONTH, Integer.toString(weekOfMonth));
+        s.data(Tags.CALENDAR_RECURRENCE_DAYOFWEEK, generateEasDayOfWeek(bareByDay));
+    }
+
     /**
      * Write recurrence information to EAS based on the RRULE in CalendarProvider
      * @param rrule the RRULE, from CalendarProvider
@@ -1048,19 +1073,17 @@ public class CalendarUtilities {
             if (freq.equals("DAILY")) {
                 s.start(Tags.CALENDAR_RECURRENCE);
                 s.data(Tags.CALENDAR_RECURRENCE_TYPE, "0");
-                s.data(Tags.CALENDAR_RECURRENCE_INTERVAL, "1");
+                addCountIntervalAndUntil(rrule, s);
-                addUntil(rrule, s);
                 s.end();
             } else if (freq.equals("WEEKLY")) {
                 s.start(Tags.CALENDAR_RECURRENCE);
                 s.data(Tags.CALENDAR_RECURRENCE_TYPE, "1");
-                s.data(Tags.CALENDAR_RECURRENCE_INTERVAL, "1");
                 // Requires a day of week (whereas RRULE does not)
+                addCountIntervalAndUntil(rrule, s);
                 String byDay = tokenFromRrule(rrule, "BYDAY=");
                 if (byDay != null) {
                     s.data(Tags.CALENDAR_RECURRENCE_DAYOFWEEK, generateEasDayOfWeek(byDay));
                 }
-                addUntil(rrule, s);
                 s.end();
             } else if (freq.equals("MONTHLY")) {
                 String byMonthDay = tokenFromRrule(rrule, "BYMONTHDAY=");
@@ -1068,35 +1091,24 @@ public class CalendarUtilities {
                     // The nth day of the month
                     s.start(Tags.CALENDAR_RECURRENCE);
                     s.data(Tags.CALENDAR_RECURRENCE_TYPE, "2");
+                    addCountIntervalAndUntil(rrule, s);
                     s.data(Tags.CALENDAR_RECURRENCE_DAYOFMONTH, byMonthDay);
-                    addUntil(rrule, s);
                     s.end();
                 } else {
                     String byDay = tokenFromRrule(rrule, "BYDAY=");
-                    String bareByDay;
                     if (byDay != null) {
-                        // This can be 1WE (1st Wednesday) or -1FR (last Friday)
-                        int wom = byDay.charAt(0);
-                        if (wom == '-') {
-                            // -1 is the only legal case (last week) Use "5" for EAS
-                            wom = 5;
-                            bareByDay = byDay.substring(2);
-                        } else {
-                            wom = wom - '0';
-                            bareByDay = byDay.substring(1);
-                        }
                         s.start(Tags.CALENDAR_RECURRENCE);
                         s.data(Tags.CALENDAR_RECURRENCE_TYPE, "3");
-                        s.data(Tags.CALENDAR_RECURRENCE_WEEKOFMONTH, Integer.toString(wom));
+                        addCountIntervalAndUntil(rrule, s);
-                        s.data(Tags.CALENDAR_RECURRENCE_DAYOFWEEK, generateEasDayOfWeek(bareByDay));
+                        addByDay(byDay, s);
-                        addUntil(rrule, s);
                         s.end();
                     }
                 }
             } else if (freq.equals("YEARLY")) {
                 String byMonth = tokenFromRrule(rrule, "BYMONTH=");
                 String byMonthDay = tokenFromRrule(rrule, "BYMONTHDAY=");
-                if (byMonth == null || byMonthDay == null) {
+                String byDay = tokenFromRrule(rrule, "BYDAY=");
+                if (byMonth == null && byMonthDay == null) {
                     // Calculate the month and day from the startDate
                     GregorianCalendar cal = new GregorianCalendar();
                     cal.setTimeInMillis(startTime);
@@ -1104,12 +1116,19 @@ public class CalendarUtilities {
                     byMonth = Integer.toString(cal.get(Calendar.MONTH) + 1);
                     byMonthDay = Integer.toString(cal.get(Calendar.DAY_OF_MONTH));
                 }
-                s.start(Tags.CALENDAR_RECURRENCE);
+                if (byMonth != null && (byMonthDay != null || byDay != null)) {
-                s.data(Tags.CALENDAR_RECURRENCE_TYPE, "5");
+                    s.start(Tags.CALENDAR_RECURRENCE);
-                s.data(Tags.CALENDAR_RECURRENCE_DAYOFMONTH, byMonthDay);
+                    s.data(Tags.CALENDAR_RECURRENCE_TYPE, byDay == null ? "5" : "6");
-                s.data(Tags.CALENDAR_RECURRENCE_MONTHOFYEAR, byMonth);
+                    addCountIntervalAndUntil(rrule, s);
-                addUntil(rrule, s);
+                    s.data(Tags.CALENDAR_RECURRENCE_MONTHOFYEAR, byMonth);
-                s.end();
+                    // Note that both byMonthDay and byDay can't be true in a valid RRULE
+                    if (byMonthDay != null) {
+                        s.data(Tags.CALENDAR_RECURRENCE_DAYOFMONTH, byMonthDay);
+                    } else {
+                        addByDay(byDay, s);
+                    }
+                    s.end();
+                }
             }
         }
     }
@@ -1131,12 +1150,12 @@ public class CalendarUtilities {
         StringBuilder rrule = new StringBuilder("FREQ=" + sTypeToFreq[type]);
 
         // INTERVAL and COUNT
-        if (interval > 0) {
-            rrule.append(";INTERVAL=" + interval);
-        }
         if (occurrences > 0) {
             rrule.append(";COUNT=" + occurrences);
         }
+        if (interval > 0) {
+            rrule.append(";INTERVAL=" + interval);
+        }
 
         // Days, weeks, months, etc.
         switch(type) {

tests/src/com/android/exchange/adapter/SyncAdapterTestCase.java

@@ -86,7 +86,7 @@ public class SyncAdapterTestCase<T extends AbstractSyncAdapter>
         return service;
     }
 
-    T getTestSyncAdapter(Class<T> klass) {
+    protected T getTestSyncAdapter(Class<T> klass) {
         EasSyncService service = getTestService();
         Constructor<T> c;
         try {

tests/src/com/android/exchange/utility/CalendarUtilitiesTests.java

@@ -22,16 +22,22 @@ import com.android.email.mail.Address;
 import com.android.email.provider.EmailContent.Account;
 import com.android.email.provider.EmailContent.Attachment;
 import com.android.email.provider.EmailContent.Message;
+import com.android.exchange.adapter.CalendarSyncAdapter;
+import com.android.exchange.adapter.Parser;
+import com.android.exchange.adapter.Serializer;
+import com.android.exchange.adapter.SyncAdapterTestCase;
+import com.android.exchange.adapter.Tags;
+import com.android.exchange.adapter.CalendarSyncAdapter.EasCalendarSyncParser;
 
 import android.content.ContentValues;
 import android.content.Entity;
 import android.content.res.Resources;
 import android.provider.Calendar.Attendees;
 import android.provider.Calendar.Events;
-import android.test.AndroidTestCase;
 import android.util.Log;
 
 import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.StringReader;
 import java.text.DateFormat;
@@ -51,7 +57,7 @@ import java.util.TimeZone;
  * http://www.ietf.org/rfc/rfc2445.txt
  */
 
-public class CalendarUtilitiesTests extends AndroidTestCase {
+public class CalendarUtilitiesTests extends SyncAdapterTestCase<CalendarSyncAdapter> {
 
     // Some prebuilt time zones, Base64 encoded (as they arrive from EAS)
     // More time zones to be added over time
@@ -612,7 +618,7 @@ public class CalendarUtilitiesTests extends AndroidTestCase {
         // Every Monday for 2 weeks
         String rrule = CalendarUtilities.rruleFromRecurrence(
                 1 /*Weekly*/, 2 /*Occurrences*/, 1 /*Interval*/, 2 /*Monday*/, 0, 0, 0, null);
-        assertEquals("FREQ=WEEKLY;INTERVAL=1;COUNT=2;BYDAY=MO", rrule);
+        assertEquals("FREQ=WEEKLY;COUNT=2;INTERVAL=1;BYDAY=MO", rrule);
         // Every Tuesday and Friday
         rrule = CalendarUtilities.rruleFromRecurrence(
                 1 /*Weekly*/, 0 /*Occurrences*/, 0 /*Interval*/, 36 /*Tue&Fri*/, 0, 0, 0, null);
@@ -639,6 +645,41 @@ public class CalendarUtilitiesTests extends AndroidTestCase {
         assertEquals("FREQ=YEARLY;BYDAY=1TU;BYMONTH=6", rrule);
     }
 
+    /**
+     * Given a CalendarSyncAdapter and an RRULE, serialize the RRULE via recurrentFromRrule and
+     * then parse the result.  Assert that the resulting RRULE is the same as the original.
+     * @param adapter a CalendarSyncAdapter
+     * @param rrule an RRULE string that will be tested
+     * @throws IOException
+     */
+    private void testSingleRecurrenceFromRrule(CalendarSyncAdapter adapter, String rrule)
+            throws IOException {
+        Serializer s = new Serializer();
+        CalendarUtilities.recurrenceFromRrule(rrule, 0, s);
+        s.done();
+        EasCalendarSyncParser parser = adapter.new EasCalendarSyncParser(
+                new ByteArrayInputStream(s.toByteArray()), adapter);
+        // The first element should be the outer CALENDAR_RECURRENCE tag
+        assertEquals(Tags.CALENDAR_RECURRENCE, parser.nextTag(Parser.START_DOCUMENT));
+        assertEquals(rrule, parser.recurrenceParser());
+    }
+
+    /**
+     * Round-trip test of RRULE handling; we serialize an RRULE and then parse the result; the
+     * result should be identical to the original RRULE
+     */
+    public void testRecurrenceFromRrule() throws IOException {
+        // A test sync adapter we can use throughout the test
+        CalendarSyncAdapter adapter = getTestSyncAdapter(CalendarSyncAdapter.class);
+
+        testSingleRecurrenceFromRrule(adapter, "FREQ=WEEKLY;COUNT=2;INTERVAL=1;BYDAY=MO");
+        testSingleRecurrenceFromRrule(adapter, "FREQ=WEEKLY;BYDAY=TU,FR");
+        testSingleRecurrenceFromRrule(adapter, "FREQ=MONTHLY;BYDAY=-1SA");
+        testSingleRecurrenceFromRrule(adapter, "FREQ=MONTHLY;BYDAY=3WE,3TH");
+        testSingleRecurrenceFromRrule(adapter, "FREQ=YEARLY;BYMONTHDAY=31;BYMONTH=10");
+        testSingleRecurrenceFromRrule(adapter, "FREQ=YEARLY;BYDAY=1TU;BYMONTH=6");
+    }
+
     /**
      * For debugging purposes, to help keep track of parsing errors.
      */